-
-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Description
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