Re: Endless loop on curl_multi_cleanup
Date: Mon, 3 Oct 2016 00:15:48 +0200
On Sun, Oct 02, 2016 at 04:41:32PM -0400, Ray Satiro via curl-library wrote:
> On 10/2/2016 4:06 PM, Dan Fandrich wrote:
> >I've discovered a case where curl_multi_cleanup() can end up in an endless
> >loop when it's called with a transfer ongoing. In such a case,
> >close_all_connections() loops around all the connections in the connection
> >cache calling Curl_disconnect() on each and not exiting until all connections
> >in the cache are closed. However, if the connection is still in use,
> >Curl_disconnect() does NOT remove the connection from the cache (the
> >Curl_disconnect, usecounter: code path) and the loop around the connection
> >cache continues, ad infinitum.
> >I'm not sure the right way to solve this. Should the forced-close bit be set on
> >all connections first to make sure they're closed? Or does whole situation
> >simply invalid because there aren't supposed to be any easy handles attached to
> >the multi handle as curl_multi_cleanup() is called? The man page doesn't
> >actually come right out and say that this isn't allowed, but does imply it.
> >This situation can occur in test 1500 when the TEST_ERR_MAJOR_BAD code path is
> >taken (such as during the memory torture test). That test doesn't actually
> >ever call curl_multi_remove_handle() as the man page recommends, and calls the
> >multi and easy cleanup functions in the opposite order to the recommendation,
> >which makes me think that other code out there doing the same and
> >curl_multi_cleanup() should handle the situation without hanging.
> Is there really something to solve here, the doc says "curl_multi_cleanup
> should be called when all easy handles are removed" .
That's what I was talking about when I said "implied". "Should" isn't the same
as "must not", so perhaps the fix is to use stronger language there. But,
given that the curl project itself got this wrong (lib1500.c) others probably
did, too, so maybe libcurl should handle this situation better. And better
might just mean logging a warning if there are still easy handles attached at
List admin: https://cool.haxx.se/list/listinfo/curl-library
Received on 2016-10-03