Menu
▾
▴
#1431 removing names from DNS cache with CURLOPT_RESOLVE is not implemented
Removing names from DNS cache with CURLOPT_RESOLVE is not implemented (see TODO in Curl_loadhostpairs). With the fix 31860ab for https://sourceforge.net/p/curl/bugs/1327/ it means that constantly adding and deleting hostcache entries with CURLOPT_RESOLVE creates a memory leak, as such entries are never removed (nor by request nor automatically).
I'm not aware of any leak, can you reproduce one for us? And feel free to help us and work on implementing the removal feature!
Imagine the application that has one multi handle. For each easy handle it will add DNS entries with CURLOPT_RESOLVE (host:port:addr) and after transfer complete - remove them with CURLOPT_RESOLVE (-host:port). In this case hostcache will grow indefinitely. This is technically not a memory leak because memory will be cleaned up when multi handle is destroyed, just that the hostcache grows large and takes more and more time to iterate through (e.g. with Curl_hostcache_prune).
As for the patch - I am not exactly sure how DNS cache locking is working.. Is it just a matter of calling Curl_resolv_unlock in loadhostpairs? I will try to make a patch..
Ah right, I understand what you mean. I wouldn't call it a "leak" but it is a waste yes and the lack of removal support is... silly. Just the result of me being a bit lazy when I wrote that code.
Curl_loadhostpairs is called in pretransfer, so doing CURLOPT_RESOLVE "-host:port" after transfer has no effect (the handle is reset before each transfer). What would be the best approach to fix it - call loadhostpairs in reset and cleanup? Doesn't sound right..
Why would you do it after the transfer? Shouldn't Curl_loadhostpairs() just act on strings that start with '-' exactly as the comment says in there?
My use case is - add host with CURLOPT_RESOLVE to dns cache, do the transfer, remove the host with CURLOPT_RESOLVE from the cache.
In libcurl setting the option just records the host to update in change list, which is promoted to dns cache by Curl_loadhostpairs which is called only in pretransfer. So setting the option CURLOPT_RESOLVE after the transfer (with intention to clear the cache) does nothing to the cache because it just records the host in the change list (which is later cleaned by reset I suppose).
I see, but I don't think that matches the description of how - works. Of course, as it isn't implemented I'll be OK with redefining the meaning.
If so, I would suggest that you make loadhostpairs create a separate removal-list for all strings that start with a dash, and then you run through that removal-list and delete the entries again when the handle is closed/removed from multi.
Sound fair?
That's why I am asking, because I do not know how it is intended to work, esp. so as it is a TODO %)
I am not sure why separate list is needed, if the user wants to change dns cache with curlopt_resolve after the transfer, then we should store everything in the cache, be it removal or adding new domains, so my suggestion would be just to run loadhostpairs when the handle is closed/removed from multi. IIUC CURLOPT_RESOLVE is special compared to other options because it manipulates not the curl handle itself, but other "global" entity (dns cache), and looks like ideal solution would be to call loadhostpairs immediately when this option is set, regardless of transfer (but maybe that's not valid because of some other considerations I have missed).
I don't think it should run when the option is set only because it would be surprise in which order things are done, and it might matter when the handle is added or not to handles or shared objects.
The different list or not question is of course a matter of how the exact implementation is done. It can be done in several different ways. Consideration needs to be made so that repeated use of a handle that has this list set works and that it works in the same defined way. The current approach that only adds names can just add the names and then forget them, but a solution that removes them again needs to be able to redo both operations on repeated use.
I am still not sure how to proceed..
See (man CURLOPT_RESOLVE) :
This means "fake" entries should influence requests through this handle only (I am not sure this is possible at all cause dns cache and "fake" entries are shared). But that would mean simply that all "fake" entries should be removed automatically without any extra CURLOPT_ setting once easy handle is closed/removed from multi. This is not the case now and this was not how I tried to fix it but that would fit my usecase perfectly.
But at the end it says :
Looks like it is implying that "fake" entries stay in that cache indefinitely independent of easy handle state until those entries are explicitly removed (this is the case now except for removal not implemented). In this case CULROPT_RESOLVE with minus entries should remove specified "fake" entries from global dns cache before transfer takes place (so with the current behaviour of dns cache it means that I will have to remove "fake" entries for the current transfer in the next transfer (because they are not removed automatically) - not very convenient). Or maybe it means that it should cancel out previous "fake" entry set with CURLOPT_RESOLVE on this handle (together with automatic removal of "fake" entries once easy handle is closed/removed)? That would make more sense. One minor question is what should be the behaviour in case of multiple interleaving CURLOPT_RESOLVE additions and removals for the same domain.
Could you please clarify which behaviour would be the most correct for the library from the user point of view and I will continue on the patch.
commit 846f4920535461 introduced CURLOPT_RESOLVE removals