Menu

#875 DNS cache - outstanding references can prevent timeout

closed-accepted
6
2013-06-21
2009-11-04
No

Heavy use of a DNS entry by multiple threads can prevent it from being ejected from the cache, even after its timeout has been exceeded. This patch attached fixes this issue.

The patch allows a DNS entry to persist outside of the hash cache and be released when the last connection referencing it calls Curl_resolv_unlock.
It also changes the hash abstraction to replace an entry with a new one by default rather than favoring older entries. I checked the other use of the hash abstraction in multi.c and it already assumes that the insert will either succeed or return NULL -- not that it will return the old element.

I've also added some debugging code that helps catch reference leaks. After curl_easy_cleanup() is called on all extant handles, curl_get_ndns() should return 0.

Discussion

  • Constantine Sapuntzakis

    Patch to fix problem

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-11-06

    Thanks a lot. This seems like a good idea.

    Two things on the patch, one question and one request:

    Question: In the function Curl_resolv_unlock() the free functions are only called in inuse is zero AND timestamp is zero. What is the rationale for checking the timestamp? If it reaches zero uses, why can't it be freed unregarding of what the time stamp field says? I think it would be suitable with a comment in the code explaining that, since it is certainly not clear to me!

    Request: the 'ndns' variable looks handy for debugging, but it should then only be included when a debug build is actually made. Use #ifdef CURLDEBUF and DEBUGF(ndns++); etc.

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-11-06
    • priority: 5 --> 6
     
  • Constantine Sapuntzakis

    Improved comments

     
  • Constantine Sapuntzakis

    New version of patch should address your concerns. Some more code cleanup too.

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-11-11

    Thanks for the report, this problem is now fixed in CVS!

     
  • Daniel Stenberg

    Daniel Stenberg - 2009-11-11
    • status: open --> closed-accepted