Navigation Menu

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

curl_easy_duphandle() drops conn_cache, negating any share handle association. #3592

Closed
sgolemon opened this issue Feb 20, 2019 · 4 comments
Closed

Comments

@sgolemon
Copy link
Contributor

sgolemon commented Feb 20, 2019

A share handle added to multiple easy handles should be sufficient to share connections between each easy, and indeed this works when each easy is created fresh and attached to a share.

However I observed that when the share handle is added to a prototype easy, then new easy handles are created from that prototype using curl_easy_duphandle(), the share appears to be "lost" and must be reattached.

Attaching the share to the easy handle produced by curl_easy_duphandle() works as expected.

I traced the cause to

curl/lib/easy.c

Line 949 in 8bc5ceb

outcurl->state.conn_cache = NULL;
which would appear to actually be a reasonable step to preserve thread-safety in the curlm case where locking callbacks are unavailable(?). If fixing curl_easy_duphandle() to preserve the conn_cache isn't possible, then at least a note or two on the manpages for the share interface and/or curl_easy_duphandle() would be helpful.

I did this

CURLSH *share = curl_share_init();
curl_share_setopt(share, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT);

CURL *base = curl_easy_init();
curl_easy_setopt(base, CURLOPT_SHARE, share);
curl_easy_setopt(base, CURLOPT_URL, "http://example.com");
// various other setopt calls

for(int i = 0; i < 10; ++i) {
  CURL *handle = curl_easy_duphandle(base);
  // child-specific setopt calls
  curl_easy_perform(handle);
  curl_easy_cleanup(handle);
}

I expected the following

One network connection to example.com, page fetched 10 times in a pipline.

curl/libcurl version

curl 7.58.0 (x86_64-pc-linux-gnu) libcurl/7.58.0 OpenSSL/1.1.0g zlib/1.2.11 libidn2/2.0.4 libpsl/0.19.1 (+libidn2/2.0.4) nghttp2/1.30.0 librtmp/2.3
Release-Date: 2018-01-24
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtmp rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IDN IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz TLS-SRP HTTP2 UnixSockets HTTPS-proxy PSL

operating system

Linux, Ubuntu 18.04

@jay
Copy link
Member

jay commented Feb 20, 2019

It says in the doc "The new handle will not inherit any state information, no connections, no SSL sessions and no cookies."

@sgolemon
Copy link
Contributor Author

Indeed, no idea how I missed that. :/

@bagder
Copy link
Member

bagder commented Feb 21, 2019

After @sgolemon's report I read the man page again, and I can see how the clearing of the share-object association isn't immediately obvious - it is just inferred by that mention of "no state", but in the share handle case the state could also be seen as held by the share handle.

Of course we can argue that this is how the function is meant to work, but then I think we should also clarify this better in the documentation. We could also easily argue that a dup'ed handle should inherit the share options that the source had set. Personally, I'm sort of leaning towards the second one. One argument against that would possibly be that we've had this behavior for so long that changing this now risks introducing something weird for people and a safer route forward would be to clarify the docs around the existing functionality.

@jay
Copy link
Member

jay commented Feb 21, 2019

The doc seems pretty clear to me.

bagder added a commit that referenced this issue Mar 1, 2019
@bagder bagder closed this as completed in 8754ddb Mar 1, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

3 participants