lib: make sigpipe handling more lazy

Define `struct Curl_sigpipe_ctx` that can be passed as argunent
to "lower" functions so that applying a transfers 'no_signal'
setting can be delayed as much as possible and sometimes avoided
alltogether.

Fixes #20326
Closes #20329
Reported-by: Dag Haavi Finstad

diff --git a/lib/conncache.c b/lib/conncache.c
index 983224722a3be8e1913451ad04f21e493468b57f..d3072fb2aa63acd47ce0d9d867e0511d2257cce3 100644 (file)
--- a/lib/conncache.c
+++ b/lib/conncache.c
@@ -235,22 +235,23 @@ void Curl_cpool_destroy(struct cpool *cpool)
 {
   if(cpool && cpool->initialised && cpool->idata) {
     struct connectdata *conn;
-    SIGPIPE_VARIABLE(pipe_st);
+    struct Curl_sigpipe_ctx pipe_ctx;
 
     CURL_TRC_M(cpool->idata, "%s[CPOOL] destroy, %zu connections",
                cpool->share ? "[SHARE] " : "", cpool->num_conn);
     /* Move all connections to the shutdown list */
-    sigpipe_init(&pipe_st);
+    sigpipe_init(&pipe_ctx);
     CPOOL_LOCK(cpool, cpool->idata);
     conn = cpool_get_first(cpool);
+    if(conn)
+      sigpipe_apply(cpool->idata, &pipe_ctx);
     while(conn) {
       cpool_remove_conn(cpool, conn);
-      sigpipe_apply(cpool->idata, &pipe_st);
       cpool_discard_conn(cpool, cpool->idata, conn, FALSE);
       conn = cpool_get_first(cpool);
     }
     CPOOL_UNLOCK(cpool, cpool->idata);
-    sigpipe_restore(&pipe_st);
+    sigpipe_restore(&pipe_ctx);
     Curl_hash_destroy(&cpool->dest2bundle);
   }
 }

diff --git a/lib/cshutdn.c b/lib/cshutdn.c
index 84db4e717ca68a5e9150749613ef1f459b2baabd..7de6c07b289efdcefff77cfd8e4930a09c1f95f8 100644 (file)
--- a/lib/cshutdn.c
+++ b/lib/cshutdn.c
@@ -176,13 +176,13 @@ static bool cshutdn_destroy_oldest(struct cshutdn *cshutdn,
   }
 
   if(e) {
-    SIGPIPE_VARIABLE(pipe_st);
+    struct Curl_sigpipe_ctx sigpipe_ctx;
     conn = Curl_node_elem(e);
     Curl_node_remove(e);
-    sigpipe_init(&pipe_st);
-    sigpipe_apply(data, &pipe_st);
+    sigpipe_init(&sigpipe_ctx);
+    sigpipe_apply(data, &sigpipe_ctx);
     Curl_cshutdn_terminate(data, conn, FALSE);
-    sigpipe_restore(&pipe_st);
+    sigpipe_restore(&sigpipe_ctx);
     return TRUE;
   }
   return FALSE;
@@ -222,7 +222,8 @@ out:
 }
 
 static void cshutdn_perform(struct cshutdn *cshutdn,
-                            struct Curl_easy *data)
+                            struct Curl_easy *data,
+                            struct Curl_sigpipe_ctx *sigpipe_ctx)
 {
   struct Curl_llist_node *e = Curl_llist_head(&cshutdn->list);
   struct Curl_llist_node *enext;
@@ -235,6 +236,7 @@ static void cshutdn_perform(struct cshutdn *cshutdn,
 
   CURL_TRC_M(data, "[SHUTDOWN] perform on %zu connections",
              Curl_llist_count(&cshutdn->list));
+  sigpipe_apply(data, sigpipe_ctx);
   while(e) {
     enext = Curl_node_next(e);
     conn = Curl_node_elem(e);
@@ -263,20 +265,19 @@ static void cshutdn_terminate_all(struct cshutdn *cshutdn,
 {
   struct curltime started = *Curl_pgrs_now(data);
   struct Curl_llist_node *e;
-  SIGPIPE_VARIABLE(pipe_st);
+  struct Curl_sigpipe_ctx sigpipe_ctx;
 
   DEBUGASSERT(cshutdn);
   DEBUGASSERT(data);
 
   CURL_TRC_M(data, "[SHUTDOWN] shutdown all");
-  sigpipe_init(&pipe_st);
-  sigpipe_apply(data, &pipe_st);
+  sigpipe_init(&sigpipe_ctx);
 
   while(Curl_llist_head(&cshutdn->list)) {
     timediff_t spent_ms;
     int remain_ms;
 
-    cshutdn_perform(cshutdn, data);
+    cshutdn_perform(cshutdn, data, &sigpipe_ctx);
 
     if(!Curl_llist_head(&cshutdn->list)) {
       CURL_TRC_M(data, "[SHUTDOWN] shutdown finished cleanly");
@@ -308,7 +309,7 @@ static void cshutdn_terminate_all(struct cshutdn *cshutdn,
   }
   DEBUGASSERT(!Curl_llist_count(&cshutdn->list));
 
-  sigpipe_restore(&pipe_st);
+  sigpipe_restore(&sigpipe_ctx);
 }
 
 int Curl_cshutdn_init(struct cshutdn *cshutdn,
@@ -418,38 +419,11 @@ void Curl_cshutdn_add(struct cshutdn *cshutdn,
              conn->connection_id, Curl_llist_count(&cshutdn->list));
 }
 
-static void cshutdn_multi_socket(struct cshutdn *cshutdn,
-                                 struct Curl_easy *data,
-                                 curl_socket_t s)
-{
-  struct Curl_llist_node *e;
-  struct connectdata *conn;
-  bool done;
-
-  DEBUGASSERT(cshutdn->multi->socket_cb);
-  e = Curl_llist_head(&cshutdn->list);
-  while(e) {
-    conn = Curl_node_elem(e);
-    if(s == conn->sock[FIRSTSOCKET] || s == conn->sock[SECONDARYSOCKET]) {
-      Curl_cshutdn_run_once(data, conn, &done);
-      if(done || cshutdn_update_ev(cshutdn, data, conn)) {
-        Curl_node_remove(e);
-        Curl_cshutdn_terminate(data, conn, FALSE);
-      }
-      break;
-    }
-    e = Curl_node_next(e);
-  }
-}
-
 void Curl_cshutdn_perform(struct cshutdn *cshutdn,
                           struct Curl_easy *data,
-                          curl_socket_t s)
+                          struct Curl_sigpipe_ctx *sigpipe_ctx)
 {
-  if((s == CURL_SOCKET_TIMEOUT) || (!cshutdn->multi->socket_cb))
-    cshutdn_perform(cshutdn, data);
-  else
-    cshutdn_multi_socket(cshutdn, data, s);
+  cshutdn_perform(cshutdn, data, sigpipe_ctx);
 }
 
 /* return fd_set info about the shutdown connections */

diff --git a/lib/cshutdn.h b/lib/cshutdn.h
index 0a15d39f1cbd67f0f98c306f645fd2eda18e33c6..79c95cca5917d8d3a8d56707707aa34c955cd696 100644 (file)
--- a/lib/cshutdn.h
+++ b/lib/cshutdn.h
@@ -30,6 +30,7 @@ struct curl_pollfds;
 struct Curl_waitfds;
 struct Curl_multi;
 struct Curl_share;
+struct Curl_sigpipe_ctx;
 
 /* Run the shutdown of the connection once.
  * Will shortly attach/detach `data` to `conn` while doing so.
@@ -97,10 +98,9 @@ void Curl_cshutdn_setfds(struct cshutdn *cshutdn,
                          fd_set *read_fd_set, fd_set *write_fd_set,
                          int *maxfd);
 
-/* Run shut down connections using socket. If socket is CURL_SOCKET_TIMEOUT,
- * run maintenance on all connections. */
+/* Run maintenance on all connections. */
 void Curl_cshutdn_perform(struct cshutdn *cshutdn,
                           struct Curl_easy *data,
-                          curl_socket_t s);
+                          struct Curl_sigpipe_ctx *sigpipe_ctx);
 
 #endif /* HEADER_CURL_CSHUTDN_H */

diff --git a/lib/easy.c b/lib/easy.c
index 4fa979c7d11af06198a01af1b0216c0ea2369202..51936660cee2cefa5906510bfbea5e2dd365c152 100644 (file)
--- a/lib/easy.c
+++ b/lib/easy.c
@@ -750,7 +750,7 @@ static CURLcode easy_perform(struct Curl_easy *data, bool events)
   struct Curl_multi *multi;
   CURLMcode mresult;
   CURLcode result = CURLE_OK;
-  SIGPIPE_VARIABLE(pipe_st);
+  struct Curl_sigpipe_ctx sigpipe_ctx;
 
   if(!data)
     return CURLE_BAD_FUNCTION_ARGUMENT;
@@ -807,8 +807,8 @@ static CURLcode easy_perform(struct Curl_easy *data, bool events)
   /* assign this after curl_multi_add_handle() */
   data->multi_easy = multi;
 
-  sigpipe_init(&pipe_st);
-  sigpipe_apply(data, &pipe_st);
+  sigpipe_init(&sigpipe_ctx);
+  sigpipe_apply(data, &sigpipe_ctx);
 
   /* run the transfer */
   result = events ? easy_events(multi) : easy_transfer(multi);
@@ -817,7 +817,7 @@ static CURLcode easy_perform(struct Curl_easy *data, bool events)
      a failure here, room for future improvement! */
   (void)curl_multi_remove_handle(multi, data);
 
-  sigpipe_restore(&pipe_st);
+  sigpipe_restore(&sigpipe_ctx);
 
   /* The multi handle is kept alive, owned by the easy handle */
   return result;
@@ -851,10 +851,10 @@ void curl_easy_cleanup(CURL *ptr)
 {
   struct Curl_easy *data = ptr;
   if(GOOD_EASY_HANDLE(data)) {
-    SIGPIPE_VARIABLE(pipe_st);
-    sigpipe_ignore(data, &pipe_st);
+    struct Curl_sigpipe_ctx sigpipe_ctx;
+    sigpipe_ignore(data, &sigpipe_ctx);
     Curl_close(&data);
-    sigpipe_restore(&pipe_st);
+    sigpipe_restore(&sigpipe_ctx);
   }
 }
 
@@ -1287,7 +1287,7 @@ CURLcode Curl_senddata(struct Curl_easy *data, const void *buffer,
 {
   CURLcode result;
   struct connectdata *c = NULL;
-  SIGPIPE_VARIABLE(pipe_st);
+  struct Curl_sigpipe_ctx sigpipe_ctx;
 
   *n = 0;
   result = easy_connection(data, &c);
@@ -1299,9 +1299,9 @@ CURLcode Curl_senddata(struct Curl_easy *data, const void *buffer,
        needs to be reattached */
     Curl_attach_connection(data, c);
 
-  sigpipe_ignore(data, &pipe_st);
+  sigpipe_ignore(data, &sigpipe_ctx);
   result = Curl_conn_send(data, FIRSTSOCKET, buffer, buflen, FALSE, n);
-  sigpipe_restore(&pipe_st);
+  sigpipe_restore(&sigpipe_ctx);
 
   if(result && result != CURLE_AGAIN)
     return CURLE_SEND_ERROR;

diff --git a/lib/multi.c b/lib/multi.c
index 594c39aff846e901a84101c6efb10ebe238c6b6a..39cf594c624e03eb62efd0a46357dd8226a2353b 100644 (file)
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -2325,7 +2325,8 @@ static CURLMcode state_connect(struct Curl_multi *multi,
 }
 
 static CURLMcode multi_runsingle(struct Curl_multi *multi,
-                                 struct Curl_easy *data)
+                                 struct Curl_easy *data,
+                                 struct Curl_sigpipe_ctx *sigpipe_ctx)
 {
   struct Curl_message *msg = NULL;
   bool connected;
@@ -2354,10 +2355,11 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
   Curl_uint32_bset_remove(&multi->dirty, data->mid);
 
   if(data == multi->admin) {
-    Curl_cshutdn_perform(&multi->cshutdn, multi->admin, CURL_SOCKET_TIMEOUT);
+    Curl_cshutdn_perform(&multi->cshutdn, multi->admin, sigpipe_ctx);
     return CURLM_OK;
   }
 
+  sigpipe_apply(data, sigpipe_ctx);
   do {
     /* A "stream" here is a logical stream if the protocol can handle that
        (HTTP/2), or the full connection for older protocols */
@@ -2731,7 +2733,7 @@ static CURLMcode multi_perform(struct Curl_multi *multi,
   CURLMcode returncode = CURLM_OK;
   struct curltime start = *multi_now(multi);
   uint32_t mid;
-  SIGPIPE_VARIABLE(pipe_st);
+  struct Curl_sigpipe_ctx sigpipe_ctx;
 
   if(multi->in_callback)
     return CURLM_RECURSIVE_API_CALL;
@@ -2739,7 +2741,8 @@ static CURLMcode multi_perform(struct Curl_multi *multi,
   if(multi->in_ntfy_callback)
     return CURLM_RECURSIVE_API_CALL;
 
-  sigpipe_init(&pipe_st);
+  sigpipe_init(&sigpipe_ctx);
+
   if(Curl_uint32_bset_first(&multi->process, &mid)) {
     CURL_TRC_M(multi->admin, "multi_perform(running=%u)",
                Curl_multi_xfers_running(multi));
@@ -2752,13 +2755,12 @@ static CURLMcode multi_perform(struct Curl_multi *multi,
         Curl_uint32_bset_remove(&multi->dirty, mid);
         continue;
       }
-      sigpipe_apply(data, &pipe_st);
-      mresult = multi_runsingle(multi, data);
+      mresult = multi_runsingle(multi, data, &sigpipe_ctx);
       if(mresult)
         returncode = mresult;
     } while(Curl_uint32_bset_next(&multi->process, mid, &mid));
   }
-  sigpipe_restore(&pipe_st);
+  sigpipe_restore(&sigpipe_ctx);
 
   if(multi_ischanged(multi, TRUE))
     process_pending_handles(multi);
@@ -3015,22 +3017,15 @@ static CURLMcode add_next_timeout(const struct curltime *pnow,
   return CURLM_OK;
 }
 
-struct multi_run_ctx {
-  struct Curl_multi *multi;
-  size_t run_xfers;
-  SIGPIPE_MEMBER(pipe_st);
-};
-
-static void multi_mark_expired_as_dirty(struct multi_run_ctx *mrc,
+static void multi_mark_expired_as_dirty(struct Curl_multi *multi,
                                         const struct curltime *ts)
 {
-  struct Curl_multi *multi = mrc->multi;
   struct Curl_easy *data = NULL;
   struct Curl_tree *t = NULL;
 
   /*
    * The loop following here will go on as long as there are expire-times left
-   * to process (compared to mrc->now) in the splay and 'data' will be
+   * to process (compared to `ts`) in the splay and 'data' will be
    * re-assigned for every expired handle we deal with.
    */
   while(1) {
@@ -3057,12 +3052,14 @@ static void multi_mark_expired_as_dirty(struct multi_run_ctx *mrc,
   }
 }
 
-static CURLMcode multi_run_dirty(struct multi_run_ctx *mrc)
+static CURLMcode multi_run_dirty(struct Curl_multi *multi,
+                                 struct Curl_sigpipe_ctx *sigpipe_ctx,
+                                 uint32_t *pnum)
 {
-  struct Curl_multi *multi = mrc->multi;
   CURLMcode mresult = CURLM_OK;
   uint32_t mid;
 
+  *pnum = 0;
   if(Curl_uint32_bset_first(&multi->dirty, &mid)) {
     do {
       struct Curl_easy *data = Curl_multi_get_easy(multi, mid);
@@ -3075,10 +3072,9 @@ static CURLMcode multi_run_dirty(struct multi_run_ctx *mrc)
           continue;
         }
 
-        mrc->run_xfers++;
-        sigpipe_apply(data, &mrc->pipe_st);
+        (*pnum)++;
         /* runsingle() clears the dirty mid */
-        mresult = multi_runsingle(multi, data);
+        mresult = multi_runsingle(multi, data, sigpipe_ctx);
 
         if(CURLM_OK >= mresult) {
           /* reassess event handling of data */
@@ -3105,12 +3101,11 @@ static CURLMcode multi_socket(struct Curl_multi *multi,
                               int *running_handles)
 {
   CURLMcode mresult = CURLM_OK;
-  struct multi_run_ctx mrc;
+  struct Curl_sigpipe_ctx pipe_ctx;
+  uint32_t run_xfers;
 
   (void)ev_bitmask;
-  memset(&mrc, 0, sizeof(mrc));
-  mrc.multi = multi;
-  sigpipe_init(&mrc.pipe_st);
+  sigpipe_init(&pipe_ctx);
 
   if(checkall) {
     /* *perform() deals with running_handles on its own */
@@ -3136,23 +3131,23 @@ static CURLMcode multi_socket(struct Curl_multi *multi,
     memset(&multi->last_expire_ts, 0, sizeof(multi->last_expire_ts));
   }
 
-  multi_mark_expired_as_dirty(&mrc, multi_now(multi));
-  mresult = multi_run_dirty(&mrc);
+  multi_mark_expired_as_dirty(multi, multi_now(multi));
+  mresult = multi_run_dirty(multi, &pipe_ctx, &run_xfers);
   if(mresult)
     goto out;
 
-  if(mrc.run_xfers) {
+  if(run_xfers) {
     /* Running transfers takes time. With a new timestamp, we might catch
      * other expires which are due now. Instead of telling the application
      * to set a 0 timeout and call us again, we run them here.
      * Do that only once or it might be unfair to transfers on other
      * sockets. */
-    multi_mark_expired_as_dirty(&mrc, &multi->now);
-    mresult = multi_run_dirty(&mrc);
+    multi_mark_expired_as_dirty(multi, &multi->now);
+    mresult = multi_run_dirty(multi, &pipe_ctx, &run_xfers);
   }
 
 out:
-  sigpipe_restore(&mrc.pipe_st);
+  sigpipe_restore(&pipe_ctx);
 
   if(multi_ischanged(multi, TRUE))
     process_pending_handles(multi);

diff --git a/lib/sigpipe.h b/lib/sigpipe.h
index 5d2c78a2713ce16419ac84f69dae1879794d9b39..2fd211c5142934e9a654707223b3a227fd794343 100644 (file)
--- a/lib/sigpipe.h
+++ b/lib/sigpipe.h
@@ -29,15 +29,12 @@
   (defined(USE_OPENSSL) || defined(USE_MBEDTLS) || defined(USE_WOLFSSL))
 #include <signal.h>
 
-struct sigpipe_ignore {
+struct Curl_sigpipe_ctx {
   struct sigaction old_pipe_act;
   BIT(no_signal);
 };
 
-#define SIGPIPE_VARIABLE(x) struct sigpipe_ignore x
-#define SIGPIPE_MEMBER(x)   struct sigpipe_ignore x
-
-static void sigpipe_init(struct sigpipe_ignore *ig)
+static CURL_INLINE void sigpipe_init(struct Curl_sigpipe_ctx *ig)
 {
   memset(ig, 0, sizeof(*ig));
   ig->no_signal = TRUE;
@@ -48,8 +45,8 @@ static void sigpipe_init(struct sigpipe_ignore *ig)
  * internals, and then sigpipe_restore() will restore the situation when we
  * return from libcurl again.
  */
-static void sigpipe_ignore(struct Curl_easy *data,
-                           struct sigpipe_ignore *ig)
+static CURL_INLINE void sigpipe_ignore(struct Curl_easy *data,
+                                       struct Curl_sigpipe_ctx *ig)
 {
   /* get a local copy of no_signal because the Curl_easy might not be
      around when we restore */
@@ -70,17 +67,17 @@ static void sigpipe_ignore(struct Curl_easy *data,
  * and SIGPIPE handling. It MUST only be called after a corresponding
  * sigpipe_ignore() was used.
  */
-static void sigpipe_restore(struct sigpipe_ignore *ig)
+static CURL_INLINE void sigpipe_restore(struct Curl_sigpipe_ctx *ig)
 {
   if(!ig->no_signal)
     /* restore the outside state */
     sigaction(SIGPIPE, &ig->old_pipe_act, NULL);
 }
 
-static void sigpipe_apply(struct Curl_easy *data,
-                          struct sigpipe_ignore *ig)
+static CURL_INLINE void sigpipe_apply(struct Curl_easy *data,
+                                      struct Curl_sigpipe_ctx *ig)
 {
-  if(data->set.no_signal != ig->no_signal) {
+  if(data && (data->set.no_signal != ig->no_signal)) {
     sigpipe_restore(ig);
     sigpipe_ignore(data, ig);
   }
@@ -88,12 +85,15 @@ static void sigpipe_apply(struct Curl_easy *data,
 
 #else
 /* for systems without sigaction */
-#define sigpipe_ignore(x, y) Curl_nop_stmt
-#define sigpipe_apply(x, y)  Curl_nop_stmt
-#define sigpipe_init(x)      Curl_nop_stmt
-#define sigpipe_restore(x)   Curl_nop_stmt
-#define SIGPIPE_VARIABLE(x)
-#define SIGPIPE_MEMBER(x)    bool x
+#define sigpipe_ignore(x, y) do { (void)x; (void)y; } while(0)
+#define sigpipe_apply(x, y)  do { (void)x; (void)y; } while(0)
+#define sigpipe_init(x)      do { (void)x; } while(0)
+#define sigpipe_restore(x)   do { (void)x; } while(0)
+
+struct Curl_sigpipe_ctx {
+  bool dummy;
+};
+
 #endif
 
 #endif /* HEADER_CURL_SIGPIPE_H */