On 10/2/2016 6:15 PM, Dan Fandrich wrote:
> 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" [1].
> 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
> that point.

Ok I see what you are saying.
1 - curl_multi_remove_handle to remove all easy handles in the multi
2 - curl_easy_cleanup can now be called at any time
3 - curl_multi_cleanup after all easy handles are removed from the multi

And something before close_all_connections(multi);
if(easyp) DEBUGF(fprintf(stderr, "curl_multi_cleanup: multi contains
easy handle(s)\n"));

-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
Received on 2016-10-03