lib: add multi_wakeup_internal

For threaded resolving, added an additional socket/eventfd pair to the
multi handle for notifications from threads. The original "double use"
of the standard wakeup pair did lead to regressions for apps.

The API definition of curl_multi_poll/wait/wakeup is pretty tight
regarding what effects what and adding notifications on top of that
broke what apps perceived to be the contract.

Fixes #22272
Reported-by: Sergei Zimmerman
Closes #22274

diff --git a/lib/asyn-thrdd.c b/lib/asyn-thrdd.c
index 1ee69f83bf03732fd4c4a4b6a696bfaffc207088..29e9b138d3f4f77ae6f4e327afe805710aa10a83 100644 (file)
--- a/lib/asyn-thrdd.c
+++ b/lib/asyn-thrdd.c
@@ -437,7 +437,7 @@ static void async_thrdd_item_process(void *arg)
 
 #endif /* HAVE_GETADDRINFO */
 
-#ifdef ENABLE_WAKEUP
+#ifdef ENABLE_INTERNAL_WAKEUP
 static void async_thrdd_event(const struct curl_thrdq *tqueue,
                               Curl_thrdq_event ev,
                               void *user_data)
@@ -446,7 +446,7 @@ static void async_thrdd_event(const struct curl_thrdq *tqueue,
   (void)tqueue;
   switch(ev) {
   case CURL_THRDQ_EV_ITEM_DONE:
-    (void)curl_multi_wakeup(multi);
+    Curl_multi_wakeup_internal(multi);
     break;
   default:
     break;
@@ -693,7 +693,7 @@ CURLcode Curl_async_pollset(struct Curl_easy *data,
 #endif
 
   if(!async->done) {
-#ifndef ENABLE_WAKEUP
+#ifndef ENABLE_INTERNAL_WAKEUP
     timediff_t stutter_ms, elapsed_ms;
     elapsed_ms = curlx_ptimediff_ms(Curl_pgrs_now(data), &async->start);
     if(elapsed_ms < 3)
@@ -736,7 +736,7 @@ CURLcode Curl_async_take_result(struct Curl_easy *data,
   if(thrdd->rr.channel)
     (void)Curl_ares_perform(thrdd->rr.channel, 0);
 #endif
-#ifndef ENABLE_WAKEUP
+#ifndef ENABLE_INTERNAL_WAKEUP
   Curl_async_thrdd_multi_process(data->multi);
 #endif
 

diff --git a/lib/multi.c b/lib/multi.c
index 7d0bc0a125c65eeb4a50c0f4d75a2cf46e37d353..265a2472276ff4400ca72f444bc0e9f39f38d158 100644 (file)
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -268,6 +268,10 @@ struct Curl_multi *Curl_multi_handle(uint32_t xfer_table_size,
   multi->wakeup_pair[0] = CURL_SOCKET_BAD;
   multi->wakeup_pair[1] = CURL_SOCKET_BAD;
 #endif
+#ifdef ENABLE_INTERNAL_WAKEUP
+  multi->wakeup_internal[0] = CURL_SOCKET_BAD;
+  multi->wakeup_internal[1] = CURL_SOCKET_BAD;
+#endif
 
   if(Curl_mntfy_resize(multi) ||
      Curl_uint32_bset_resize(&multi->process, xfer_table_size) ||
@@ -315,6 +319,10 @@ struct Curl_multi *Curl_multi_handle(uint32_t xfer_table_size,
   if(Curl_wakeup_init(multi->wakeup_pair, TRUE) < 0)
     goto error;
 #endif
+#ifdef ENABLE_INTERNAL_WAKEUP
+  if(Curl_wakeup_init(multi->wakeup_internal, TRUE) < 0)
+    goto error;
+#endif
 
   if(Curl_probeipv6(multi))
     goto error;
@@ -360,6 +368,9 @@ error:
 #ifdef ENABLE_WAKEUP
   Curl_wakeup_destroy(multi->wakeup_pair);
 #endif
+#ifdef ENABLE_INTERNAL_WAKEUP
+  Curl_wakeup_destroy(multi->wakeup_internal);
+#endif
 
   curlx_free(multi);
   return NULL;
@@ -395,7 +406,7 @@ bool Curl_is_connecting(struct Curl_easy *data)
 
 static CURLMcode multi_assess_wakeup(struct Curl_multi *multi)
 {
-#ifdef ENABLE_WAKEUP
+#ifdef ENABLE_INTERNAL_WAKEUP
   if(multi->socket_cb)
     return Curl_multi_ev_assess_xfer(multi, multi->admin);
 #else
@@ -1148,14 +1159,14 @@ CURLMcode Curl_multi_pollset(struct Curl_easy *data,
   CURLcode result = CURLE_OK;
 
   Curl_pollset_reset(ps);
-#ifdef ENABLE_WAKEUP
+#ifdef ENABLE_INTERNAL_WAKEUP
   /* The admin handle always listens on the wakeup socket when there
    * are transfers alive. */
   if(data->multi && (data == data->multi->admin) &&
      data->multi->xfers_really_alive) {
     CURL_TRC_M(data, "adding wakeup, %u xfers really alive",
                data->multi->xfers_really_alive);
-    result = Curl_pollset_add_in(data, ps, data->multi->wakeup_pair[0]);
+    result = Curl_pollset_add_in(data, ps, data->multi->wakeup_internal[0]);
   }
 #endif
   /* If the transfer has no connection, this is fine. Happens when
@@ -1713,6 +1724,18 @@ CURLMcode curl_multi_wakeup(CURLM *m)
   return mresult;
 }
 
+#ifdef ENABLE_INTERNAL_WAKEUP
+void Curl_multi_wakeup_internal(struct Curl_multi *multi)
+{
+  /* This is expected to be invocable from another thread which
+   * does NOT outlive the multi handle. Check for sanity. */
+  if(GOOD_MULTI_HANDLE(multi))
+    Curl_wakeup_signal(multi->wakeup_internal);
+  else
+    DEBUGASSERT(0);
+}
+#endif
+
 /*
  * multi_ischanged() is called
  *
@@ -2739,10 +2762,10 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
   Curl_uint32_bset_remove(&multi->dirty, data->mid);
 
   if(data == multi->admin) {
-#ifdef ENABLE_WAKEUP
+#ifdef ENABLE_INTERNAL_WAKEUP
     /* Consume any pending wakeup signals before processing.
      * This is necessary for event based processing. See #21547 */
-    (void)Curl_wakeup_consume(multi->wakeup_pair, TRUE);
+    (void)Curl_wakeup_consume(multi->wakeup_internal, TRUE);
 #endif
 #ifdef USE_RESOLV_THREADED
     Curl_async_thrdd_multi_process(multi);
@@ -3050,6 +3073,9 @@ CURLMcode curl_multi_cleanup(CURLM *m)
 #ifdef ENABLE_WAKEUP
   Curl_wakeup_destroy(multi->wakeup_pair);
 #endif
+#ifdef ENABLE_INTERNAL_WAKEUP
+  Curl_wakeup_destroy(multi->wakeup_internal);
+#endif
 
     multi_xfer_bufs_free(multi);
     Curl_mntfy_cleanup(multi);

diff --git a/lib/multihandle.h b/lib/multihandle.h
index 5cf2c640e547ac2bf734f23e8e7d23136a2828f6..1af07fa02ba456a4f07061bc12b9d16c317621bf 100644 (file)
--- a/lib/multihandle.h
+++ b/lib/multihandle.h
@@ -77,6 +77,11 @@ typedef enum {
 #if !defined(CURL_DISABLE_SOCKETPAIR) && !defined(USE_WINSOCK)
 #define ENABLE_WAKEUP
 #endif
+#if !defined(CURL_DISABLE_SOCKETPAIR) && \
+    defined(USE_RESOLV_THREADED) && \
+    !defined(USE_WINSOCK)
+#define ENABLE_INTERNAL_WAKEUP
+#endif
 
 /* value for MAXIMUM CONCURRENT STREAMS upper limit */
 #define INITIAL_MAX_CONCURRENT_STREAMS ((1U << 31) - 1)
@@ -172,7 +177,13 @@ struct Curl_multi {
 #ifdef ENABLE_WAKEUP
   curl_socket_t wakeup_pair[2]; /* eventfd()/pipe()/socketpair() used for
                                    wakeup 0 is used for read, 1 is used
-                                   for write */
+                                   for write. Used by curl_multi_wakeup() */
+#endif
+#ifdef ENABLE_INTERNAL_WAKEUP
+  curl_socket_t wakeup_internal[2]; /* eventfd()/pipe()/socketpair() used for
+                                   wakeup 0 is used for read, 1 is used
+                                   for write. Used for internal wakeups,
+                                   e.g. threaded resolver. */
 #endif
   unsigned int max_concurrent_streams;
   unsigned int maxconnects; /* if >0, a fixed limit of the maximum number of

diff --git a/lib/multiif.h b/lib/multiif.h
index 7fad9ca33e4f01949d46243681c775b25a8f3cae..023d3253aac16fc789a62b9ad9d13a3322d8c33c 100644 (file)
--- a/lib/multiif.h
+++ b/lib/multiif.h
@@ -164,4 +164,8 @@ void Curl_multi_clear_dirty(struct Curl_easy *data);
 
 void Curl_multi_set_now(struct Curl_multi *multi);
 
+#ifdef ENABLE_INTERNAL_WAKEUP
+void Curl_multi_wakeup_internal(struct Curl_multi *multi);
+#endif
+
 #endif /* HEADER_CURL_MULTIIF_H */

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index e88472728b23d07f28e923574e4bded4951aa9ab..32ad4b1e522cb745e77166c2e74329a2138ed4ba 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -263,7 +263,7 @@ test2300 test2301 test2302 test2303 test2304 test2305 test2306 test2307 test2308
 test2309 test2310 test2311 \
 \
 test2400 test2401 test2402 test2403 test2404 test2405 test2406 test2407 \
-test2408 test2409 test2410 test2411 test2412 test2413 \
+test2408 test2409 test2410 test2411 test2412 test2413 test2414 \
 \
 test2500 test2501 test2502 test2503 test2504 test2505 test2506 \
 \

diff --git a/tests/data/test2414 b/tests/data/test2414
new file mode 100644 (file)
index 0000000..197beff
--- /dev/null
+++ b/tests/data/test2414
@@ -0,0 +1,30 @@
+<?xml version="1.0" encoding="US-ASCII"?>
+<testcase>
+<info>
+<keywords>
+multi
+wakeup
+</keywords>
+</info>
+
+# Client-side
+<client>
+<features>
+wakeup
+</features>
+<server>
+</server>
+<tool>
+lib%TESTNUMBER
+</tool>
+<name>
+wakeup, perform and poll needs to exit
+</name>
+<command>
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+</verify>
+</testcase>

diff --git a/tests/libtest/Makefile.inc b/tests/libtest/Makefile.inc
index 07d78b51925f9e8fcf671cbd20f886c9a407cbf7..7b959e0df4988e462fd4b7d0b7401220fa5c18ab 100644 (file)
--- a/tests/libtest/Makefile.inc
+++ b/tests/libtest/Makefile.inc
@@ -119,7 +119,7 @@ TESTS_C = \
   lib2023.c lib2032.c lib2082.c \
   lib2301.c lib2302.c lib2304.c           lib2306.c lib2308.c lib2309.c \
   lib2402.c           lib2404.c lib2405.c \
-  lib2412.c \
+  lib2412.c           lib2414.c \
   lib2502.c lib2504.c lib2505.c lib2506.c \
   lib2700.c \
   lib3010.c lib3025.c lib3026.c lib3027.c lib3033.c lib3034.c \

diff --git a/tests/libtest/lib2414.c b/tests/libtest/lib2414.c
new file mode 100644 (file)
index 0000000..e04dff3
--- /dev/null
+++ b/tests/libtest/lib2414.c
@@ -0,0 +1,53 @@
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) Dmitry Karpov <dkarpov1970@gmail.com>
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at https://curl.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ * SPDX-License-Identifier: curl
+ *
+ ***************************************************************************/
+
+#include "first.h"
+#include "testtrace.h"
+
+static CURLcode test_lib2414(const char *URL)
+{
+  CURLM *multi = NULL;
+  CURLcode result = CURLE_OK;
+  int running;
+
+  (void)URL;
+  global_init(CURL_GLOBAL_ALL);
+
+  multi = curl_multi_init();
+  if(!multi) {
+    curl_mfprintf(stderr, "curl_multi_init() failed\n");
+    result = TEST_ERR_MAJOR_BAD;
+    goto test_cleanup;
+  }
+
+  curl_multi_wakeup(multi);
+  curl_multi_perform(multi, &running);
+  curl_multi_poll(multi, NULL, 0, INT_MAX, NULL);
+
+test_cleanup:
+  if(multi)
+    curl_multi_cleanup(multi);
+  curl_global_cleanup();
+  return result;
+}