-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
curl_multi_socket_action with large posts won't multiplex properly #3436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Some further debugging shows that when I get the fd_set from curl_multi_fdset, it differs from the ones passed to me via |
Just to be sure this isn't a recent regression: this is reproducible on 7.55.1 on Ubuntu (but also 7.63.0 on CentOS as indicated in the initial post). |
The code looks like it uses libcurl wrongly. It uses a mix of event-based and non event-based functions in a way I'm not convinced is fine. It should wait only for the sockets told about by the socket callback and it should not use |
Thanks for checking. I'm not using The timeout logic is indeed broken but should be sufficient for the purpose of demonstrating the issue. |
And exactly how sure are we that this sleep only affects one particular stream and not the whole connection? |
That's a fair question. Here's the output of h2load:
In other words, the server is able to handle 10 concurrent requests in 1 second, despite all being executed over the same connection. Each request takes one second. The code I posted ( Swapping the event loop code for |
I've figured it out. Now I just need to fix it too ... =) |
Fixes #3436 After LOTS of scratching my head, I eventually realized that even when doing 10 uploads in parallel, sometimes the socket callback to the application that tells it what to wait for on the socket, looked like it would reflect the status of just the single transfer that just changed state. Digging into the code revealed that this was indeed the truth. When multiple transfers are using the same connection, the application did not correctly get the *combined* flags for all transfers which then could make it switch to READ (only) when in fact most transfers wanted to get told when the socket was WRITEABLE. A separate but related regression had also been introduced by me when I cleared connection/transfer association better a while ago, as now the logic couldn't find the connection and see if that was marked as used by more transfers and then it would also prematurely remove the socket from the socket hash table even in times other transfers were still using it! Make sure that each socket stored in the socket hash has a "combined" action field of what to ask the application to wait for, that is potentially the ORed action of multiple parallel transfers. And remove that socket hash entry only if there are no transfers left using it. The socket hash entry stored an association to a single transfer using that socket - and when curl_multi_socket_action() was called to tell libcurl about activities on that specific socket only that transfer was "handled". This was WRONG, as a single socket/connection can be used by numerous parallel transfers and not necessarily a single one. We now store a list of handles in the socket hashtable entry and when libcurl is told there's traffic for a particular socket, it now iterates over all known transfers using that single socket.
Fixes #3436 Problem 1 After LOTS of scratching my head, I eventually realized that even when doing 10 uploads in parallel, sometimes the socket callback to the application that tells it what to wait for on the socket, looked like it would reflect the status of just the single transfer that just changed state. Digging into the code revealed that this was indeed the truth. When multiple transfers are using the same connection, the application did not correctly get the *combined* flags for all transfers which then could make it switch to READ (only) when in fact most transfers wanted to get told when the socket was WRITEABLE. Problem 1b A separate but related regression had also been introduced by me when I cleared connection/transfer association better a while ago, as now the logic couldn't find the connection and see if that was marked as used by more transfers and then it would also prematurely remove the socket from the socket hash table even in times other transfers were still using it! Fix 1 Make sure that each socket stored in the socket hash has a "combined" action field of what to ask the application to wait for, that is potentially the ORed action of multiple parallel transfers. And remove that socket hash entry only if there are no transfers left using it. Problem 2 The socket hash entry stored an association to a single transfer using that socket - and when curl_multi_socket_action() was called to tell libcurl about activities on that specific socket only that transfer was "handled". This was WRONG, as a single socket/connection can be used by numerous parallel transfers and not necessarily a single one. Fix 2 We now store a list of handles in the socket hashtable entry and when libcurl is told there's traffic for a particular socket, it now iterates over all known transfers using that single socket.
I did this
I'm performing multiple concurrent requests to a HTTP/2 server, each with a large POST body. The server execution takes exactly 1 second (because
sleep(1000)
), and can run many in parallel. Usingcurl_multi_perform
, this takes one second as expected. Usingcurl_multi_socket_action
, it takes one second per request.Code: https://gist.github.com/TvdW/c41cb1e74feb0c5f79be83f9d330a74b - argv[1] refers to the server to test with, I've been using https://home.xifon.eu/sleep/1000 which runs a http2 server that sleeps for exactly one second. When commenting out the "BAD" section (thus allowing the "GOOD" section to run), the difference can be seen.
I expected the following
Doing 10 concurrent requests that each take one second should take one second total.
curl/libcurl version
operating system
CentOS 7
The text was updated successfully, but these errors were encountered: