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

CURLOPT_NOSIGNAL does not work everywhere #3138

Closed
martinus opened this issue Oct 15, 2018 · 5 comments
Closed

CURLOPT_NOSIGNAL does not work everywhere #3138

martinus opened this issue Oct 15, 2018 · 5 comments
Assignees

Comments

@martinus
Copy link

martinus commented Oct 15, 2018

I did this

I use the curl_easy interfaces in 2 threads, and immediately call curl_easy_setopt(curl, CURLOPT_NOSIGNAL);. Eventually, in curl_easy_cleanup the instances are destroyed, where unfortunately sigaction(SIGPIPE, &action, NULL); is called and in our case due to race condition this keeps the SIG_IGN installed if both destructions happen at almost the same time.

Here is the relevant stacktrace:

#0  0xfffffd7fff082824 in sigaction () from /lib/64/libc.so.1
#1  0x000000000095837a in sigpipe_ignore (ig=ig@entry=0xfffffd7ffe6001e0, data=<optimized out>) at sigpipe.h:51
#2  0x0000000000958ee9 in sigpipe_ignore (data=<optimized out>, ig=0xfffffd7ffe6001e0) at conncache.c:604
#3  Curl_conncache_close_all_connections (connc=connc@entry=0x12d0738) at conncache.c:597
#4  0x0000000000952220 in curl_multi_cleanup (multi=0x12d0640) at multi.c:2219
#5  0x000000000096ff8d in Curl_close (data=data@entry=0x12c6f70) at url.c:326
#6  0x000000000094e797 in curl_easy_cleanup (data=0x12c6f70) at easy.c:832

I expected the following

sigaction()should never be called when we use curl_easy_setopt(curl, CURLOPT_NOSIGNAL);

curl/libcurl version

curl-7.61.1

operating system

Solaris, Linux

@martinus
Copy link
Author

I have a small patch that seems to work for us:

--- lib/easy.c	2018-10-15 10:43:12.715335746 +0200
+++ lib/easy.c	2018-10-15 10:43:22.039433726 +0200
@@ -761,6 +761,8 @@
   /* Copy the MAXCONNECTS option to the multi handle */
   curl_multi_setopt(multi, CURLMOPT_MAXCONNECTS, data->set.maxconnects);
 
+  multi->conn_cache.closure_handle->set.no_signal = data->set.no_signal;
+
   mcode = curl_multi_add_handle(multi, data);
   if(mcode) {
     curl_multi_cleanup(multi);

I'm not sure though if that is the best place to do this.

@bagder
Copy link
Member

bagder commented Oct 16, 2018

Thanks for the report, I think you're entirely correct and I'll give your suggested a fix a closer look...

@bagder
Copy link
Member

bagder commented Oct 18, 2018

@martinus: I just put the assignment in another place, next to two other options that also get "inherited" by the closure handle. I'll appreciate if you can just verify that this also works for you. See #3147!

@martinus
Copy link
Author

I've just tried this your fix and it works for us. Thanks!

@bagder
Copy link
Member

bagder commented Oct 19, 2018

Excellent, thanks for confirming!

@bagder bagder closed this as completed in 8a49f91 Oct 19, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jan 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants