I did this
Hello,
we are using libcurl to do rather standard HTTP connections. However, we need to handle sockets ourselves and we use CURLMOPT_SOCKETFUNCTION callback that manages our sockets. In this callback we call curl_multi_assign to keep track of the sockets in our internal structures. The code broke recently because of the way curl_multi_cleanup works.
To reproduce the issue, we need to build cURL with --enable-debug --enable-curldebug to have GOOD_MULTI_HANDLE check active.
The issue is related to recent cpool changes but I don't believe it's actual culprit. When curl_multi_cleanup is called, it immediately invalidates multi handle magic. However it doesn't take into account that user callbacks can still be called with that multi handle and those callbacks may call curl_multi_assign (it is permissible to call this function from a callback).
This behavior is forced by a call to Curl_cpool_destroy from within curl_multi_cleanup. It attempts to shut down all existing connections in the connection pool (the connections without an easy handle attached invisible to the application). Interestingly enough, it later calls socket callback with CURL_POLL_IN (and later with CURL_POLL_REMOVE).
I don't have enough introspection as to why cpool is trying to listen on a socket but I believe that until Curl_cpool_destroy completes, we should not invalidate multi handle's magic.
Unless I am missing something, I propose following patch:
--- a/lib/multi.c 2024-10-08 07:08:56.000000000 -0600
+++ a/lib/multi.c 2024-10-08 07:09:47.839271207 -0600
@@ -2726,8 +2726,6 @@
if(multi->in_callback)
return CURLM_RECURSIVE_API_CALL;
- multi->magic = 0; /* not good anymore */
-
/* move the pending and msgsent entries back to process
so that there is just one list to iterate over */
unlink_all_msgsent_handles(multi);
@@ -2761,6 +2759,8 @@
Curl_cpool_destroy(&multi->cpool);
+ multi->magic = 0; /* not good anymore */
+
sockhash_destroy(&multi->sockhash);
Curl_hash_destroy(&multi->proto_hash);
Curl_hash_destroy(&multi->hostcache);
I expected the following
I expect that following "callstack" would work properly:
curl_multi_cleanup()
-- Curl_cpool_destroy()
-- -- _socketaction_callback()
-- -- -- curl_multi_assign() # doesn't assert
curl/libcurl version
curl 8.10.1
operating system
Ubuntu 22, custom cURL build
I did this
Hello,
we are using libcurl to do rather standard HTTP connections. However, we need to handle sockets ourselves and we use
CURLMOPT_SOCKETFUNCTIONcallback that manages our sockets. In this callback we callcurl_multi_assignto keep track of the sockets in our internal structures. The code broke recently because of the waycurl_multi_cleanupworks.To reproduce the issue, we need to build cURL with
--enable-debug --enable-curldebugto haveGOOD_MULTI_HANDLEcheck active.The issue is related to recent
cpoolchanges but I don't believe it's actual culprit. Whencurl_multi_cleanupis called, it immediately invalidates multi handle magic. However it doesn't take into account that user callbacks can still be called with that multi handle and those callbacks may callcurl_multi_assign(it is permissible to call this function from a callback).This behavior is forced by a call to
Curl_cpool_destroyfrom withincurl_multi_cleanup. It attempts to shut down all existing connections in the connection pool (the connections without an easy handle attached invisible to the application). Interestingly enough, it later calls socket callback withCURL_POLL_IN(and later withCURL_POLL_REMOVE).I don't have enough introspection as to why cpool is trying to listen on a socket but I believe that until
Curl_cpool_destroycompletes, we should not invalidate multi handle's magic.Unless I am missing something, I propose following patch:
I expected the following
I expect that following "callstack" would work properly:
curl/libcurl version
curl 8.10.1
operating system
Ubuntu 22, custom cURL build