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

Comments

Projects
None yet
2 participants
@martinus

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

This comment has been minimized.

martinus commented Oct 15, 2018

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

martinus commented Oct 19, 2018

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

@bagder

This comment has been minimized.

Member

bagder commented Oct 19, 2018

Excellent, thanks for confirming!

@bagder bagder closed this in 8a49f91 Oct 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment