curl_multi_remove_handle() can block until DNS resolution finishes even when using c-ares

[b-spencer]

This is related to #2975, but it's not exactly the same issue.

When configured to use c-ares as the DNS resolver, and a handle is still waiting for DNS resolution (such as when the DNS server is unreachable), the curl_multi_remove_handle() call can block until that DNS request times out.

This behaviour appears to have been introduced as part of ac9a179.

I understand that it might not be possible to cancel an in-progress DNS request with all resolver backends without blocking. It does seem like it should be possible to cancel an in-progress c-ares DNS request without blocking.

Changing the multi_done() call of Curl_resolver_wait_resolv() to Curl_resolver_cancel() seems to work, but I'm not familiar enough with the details of resolver management inside cURL (especially for other resolver backends) to know if this is correct or "safe" in all cases. Based on the discussion in #2975, it sounds like the threaded resolver must join with the thread.

Perhaps Curl_resolver_cancel() should take a flag that says whether it must not return until all resources are freed. The c-ares resolver could then simply ignore the flag since has nothing special to do, then clean up and return quickly, while the threaded resolver could block until it has joined with the thread.

(BTW, it looks like there is at least one case where Curl_resolver_cancel() is called when it "shouldn't" wait, in conn_free(), but I'm not sure how the threaded resolver's threads get cleaned up in that case.)

[bagder]

Agreed. The c-ares backend shouldn't wait for completion for resolves it doesn't care about anymore and it doesn't have to do it.

We could even just make it more explicit and provide a Curl_resolver_killall function in the resolver backends that would do the right thing, as you describe.

Would you like to work on a fix for this?

[b-spencer]

I will take a look, yes, but I might not be able to publish a patch until January. There are only the two resolvers to worry about, right? asyn-ares and asyn-thread?

[bagder]

Right. The third one is the synchronous backend but that can probably be implemented just as an empty macro or something since it will never actually end up in that state when used.