Skip to content
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

Documentation about CURL_POLL_REMOVE is a bit misleading #9799

Closed
lultimouomo opened this issue Oct 25, 2022 · 3 comments
Closed

Documentation about CURL_POLL_REMOVE is a bit misleading #9799

lultimouomo opened this issue Oct 25, 2022 · 3 comments

Comments

@lultimouomo
Copy link

I am using a hiper-style integration of curl with asio. To do this, I set a socket callback using CURLMOPT_SOCKETFUNCTION and a pair of socket open/close callbacks using CURLOPT_OPENSOCKETFUNCTION and CURLOPT_CLOSESOCKETFUNCTION.

I noticed that when sending several requests to an HTTP/1.1 server only a couple of sockets are opened with my OPENSOCKETFUNCTION, which are then used for multiple requests. For each socket I receive several times a socket callback with the what parameter set to CURL_POLL_REMOVE. I believe what is happening is that after each request is completed cURL invokes the socket callback with CURL_POLL_REMOVE, then puts the socket in an idle connection pool, then reuses it for the next request to the same server. This makes sense, but is not what I was expecting from the documentation of CURLMOPT_SOCKETFUNCTION, which says:

CURL_POLL_REMOVE The specified socket/file descriptor is no longer used by libcurl.

Now, the documentation does not say that it won't ever be used again, but it kind of gives that impression. Maybe the documentation could be clarified a bit by saying that cURL might not close that socket and try to use it again later on? Unfortunately there isn't any code example using both CURLMOPT_SOCKETFUNCTION and CURLOPT_OPENSOCKETFUNCTION, and there is not much documentation about their interaction. I was under the impression that a socket would always be closed after CURL_POLL_REMOVE, while clearly this is not the case.

@bagder
Copy link
Member

bagder commented Oct 25, 2022

CURL_POLL_REMOVE The specified socket/file descriptor is no longer used by libcurl.

I suppose we could add "for any active transfer". it is removed from the set of sockets that need to be monitored by the application. It can of course be added again once it needs monitoring again. Do you have any other alternative phrasing in mind that you think might have helped you understand this better?

The open/close callbacks are totally separate and independent from the socket callback.

@bch
Copy link
Contributor

bch commented Oct 25, 2022 via email

bagder added a commit that referenced this issue Oct 25, 2022
The removal is brief or long, don't assume.

Reported-by: Luca Niccoli

Fixes #9799
Closes #....
@lultimouomo
Copy link
Author

The open/close callbacks are totally separate and independent from the socket callback.

Yes, but they both involve some bookkeeping about the sockets, and since there are no examples about how to use the socket callback and the open/close callbacks together I somehow got that bookkeeping mixed up, thinking I could get rid of the socket after CURL_POLL_REMOVE (since the socket was "no longer used by libcurl"). Which, incidentally, usually works, because curl will try to use the socket descriptor, see that it it's not working, call the close function and then open a new one - but this prevents connection reuse and also will break, I guess, if the same socket descriptor is reused by the operating system.

The documentation change makes it a bit easier to get this right.

@bagder bagder closed this as completed in 716ad5e Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants