curl_multi_poll: a sister to curl_multi_wait() that waits morebagder/curl_multi_poll

Repeatedly we see problems where using curl_multi_wait() is difficult or
just awkward because if it has no file descriptor to wait for
internally, it returns immediately and leaves it to the caller to wait
for a small amount of time in order to avoid occasional busy-looping.

This is often missed or misunderstood, leading to underperforming
applications.

This change introduces curl_multi_poll() as a replacement drop-in
function that accepts the exact same set of arguments. This function
works identically to curl_multi_wait() - EXCEPT - for the case when
there's nothing to wait for internally, as then this function will by
itself wait for a "suitable" short time before it returns. This
effectiely avoids all risks of busy-looping and should also make it less
likely that apps "over-wait".

This also changes the curl tool to use this funtion internally when
doing parallel transfers and changes curl_easy_perform() to use it
internally.

9 files changed, 161 insertions, 80 deletions

diff --git a/docs/libcurl/Makefile.inc b/docs/libcurl/Makefile.inc
index b4ff45dde..bd88c9c38 100644
--- a/docs/libcurl/Makefile.inc
+++ b/docs/libcurl/Makefile.inc
@@ -46,6 +46,7 @@ man_MANS = \
  curl_multi_info_read.3 \
  curl_multi_init.3 \
  curl_multi_perform.3 \
+ curl_multi_poll.3 \
  curl_multi_remove_handle.3 \
  curl_multi_setopt.3 \
  curl_multi_socket.3 \
diff --git a/docs/libcurl/curl_multi_poll.3 b/docs/libcurl/curl_multi_poll.3
new file mode 100644
index 000000000..9fc72c55d
--- /dev/null
+++ b/docs/libcurl/curl_multi_poll.3
@@ -0,0 +1,110 @@
+.\" **************************************************************************
+.\" *                                  _   _ ____  _
+.\" *  Project                     ___| | | |  _ \| |
+.\" *                             / __| | | | |_) | |
+.\" *                            | (__| |_| |  _ <| |___
+.\" *                             \___|\___/|_| \_\_____|
+.\" *
+.\" * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
+.\" *
+.\" * 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.haxx.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.
+.\" *
+.\" **************************************************************************
+.TH curl_multi_poll 3 "29 Jul 2019" "libcurl 7.66.0" "libcurl Manual"
+.SH NAME
+curl_multi_poll - polls on all easy handles in a multi handle
+.SH SYNOPSIS
+.nf
+#include <curl/curl.h>
+
+CURLMcode curl_multi_poll(CURLM *multi_handle,
+                          struct curl_waitfd extra_fds[],
+                          unsigned int extra_nfds,
+                          int timeout_ms,
+                          int *numfds);
+.ad
+.SH DESCRIPTION
+\fIcurl_multi_poll(3)\fP polls all file descriptors used by the curl easy
+handles contained in the given multi handle set.  It will block until activity
+is detected on at least one of the handles or \fItimeout_ms\fP has passed.
+Alternatively, if the multi handle has a pending internal timeout that has a
+shorter expiry time than \fItimeout_ms\fP, that shorter time will be used
+instead to make sure timeout accuracy is reasonably kept.
+
+The calling application may pass additional curl_waitfd structures which are
+similar to \fIpoll(2)\fP's pollfd structure to be waited on in the same call.
+
+On completion, if \fInumfds\fP is non-NULL, it will be populated with the
+total number of file descriptors on which interesting events occurred. This
+number can include both libcurl internal descriptors as well as descriptors
+provided in \fIextra_fds\fP.
+
+If no extra file descriptors are provided and libcurl has no file descriptor
+to offer to wait for, this function will instead wait during \fItimeout_ms\fP
+milliseconds (or shorter if an internal timer indicates so). This is the
+detail that makes this function different than \fIcurl_multi_wait(3)\fP.
+
+This function is encouraged to be used instead of select(3) when using the
+multi interface to allow applications to easier circumvent the common problem
+with 1024 maximum file descriptors.
+.SH curl_waitfd
+.nf
+struct curl_waitfd {
+  curl_socket_t fd;
+  short events;
+  short revents;
+};
+.fi
+.IP CURL_WAIT_POLLIN
+Bit flag to curl_waitfd.events indicating the socket should poll on read
+events such as new data received.
+.IP CURL_WAIT_POLLPRI
+Bit flag to curl_waitfd.events indicating the socket should poll on high
+priority read events such as out of band data.
+.IP CURL_WAIT_POLLOUT
+Bit flag to curl_waitfd.events indicating the socket should poll on write
+events such as the socket being clear to write without blocking.
+.SH EXAMPLE
+.nf
+CURL *easy_handle;
+CURLM *multi_handle;
+
+/* add the individual easy handle */
+curl_multi_add_handle(multi_handle, easy_handle);
+
+do {
+  CURLMcode mc;
+  int numfds;
+
+  mc = curl_multi_perform(multi_handle, &still_running);
+
+  if(mc == CURLM_OK) {
+    /* wait for activity or timeout */
+    mc = curl_multi_poll(multi_handle, NULL, 0, 1000, &numfds);
+  }
+
+  if(mc != CURLM_OK) {
+    fprintf(stderr, "curl_multi failed, code %d.\\n", mc);
+    break;
+  }
+
+} while(still_running);
+
+curl_multi_remove_handle(multi_handle, easy_handle);
+.fi
+.SH RETURN VALUE
+CURLMcode type, general libcurl multi interface error code. See
+\fIlibcurl-errors(3)\fP
+.SH AVAILABILITY
+This function was added in libcurl 7.66.0.
+.SH "SEE ALSO"
+.BR curl_multi_fdset "(3), " curl_multi_perform "(3), " curl_multi_wait "(3)"
diff --git a/include/curl/multi.h b/include/curl/multi.h
index b19dbaf79..6777b446f 100644
--- a/include/curl/multi.h
+++ b/include/curl/multi.h
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -173,6 +173,20 @@ CURL_EXTERN CURLMcode curl_multi_wait(CURLM *multi_handle,
                                       int timeout_ms,
                                       int *ret);
 
+/*
+ * Name:     curl_multi_poll()
+ *
+ * Desc:     Poll on all fds within a CURLM set as well as any
+ *           additional fds passed to the function.
+ *
+ * Returns:  CURLMcode type, general multi error code.
+ */
+CURL_EXTERN CURLMcode curl_multi_poll(CURLM *multi_handle,
+                                      struct curl_waitfd extra_fds[],
+                                      unsigned int extra_nfds,
+                                      int timeout_ms,
+                                      int *ret);
+
  /*
   * Name:    curl_multi_perform()
   *
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 7c258b3fa..b9375e6ba 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -65,7 +65,7 @@ endif
 # Prevent LIBS from being used for all link targets
 LIBS = $(BLANK_AT_MAKETIME)
 
-VERSIONINFO=-version-info 9:0:5
+VERSIONINFO=-version-info 10:0:6
 # This flag accepts an argument of the form current[:revision[:age]]. So,
 # passing -version-info 3:12:1 sets current to 3, revision to 12, and age to
 # 1.
diff --git a/lib/easy.c b/lib/easy.c
index 616ad28b8..b8d94c740 100644
--- a/lib/easy.c
+++ b/lib/easy.c
@@ -602,27 +602,11 @@ static CURLcode easy_transfer(struct Curl_multi *multi)
 
   while(!done && !mcode) {
     int still_running = 0;
-    bool gotsocket = FALSE;
-
-    mcode = Curl_multi_wait(multi, NULL, 0, 1000, NULL, &gotsocket);
-
-    if(!mcode) {
-      if(!gotsocket) {
-        long sleep_ms;
-
-        /* If it returns without any filedescriptor instantly, we need to
-           avoid busy-looping during periods where it has nothing particular
-           to wait for */
-        curl_multi_timeout(multi, &sleep_ms);
-        if(sleep_ms) {
-          if(sleep_ms > 1000)
-            sleep_ms = 1000;
-          Curl_wait_ms((int)sleep_ms);
-        }
-      }
 
+    mcode = curl_multi_poll(multi, NULL, 0, 1000, NULL);
+
+    if(!mcode)
       mcode = curl_multi_perform(multi, &still_running);
-    }
 
     /* only read 'still_running' if curl_multi_perform() return OK */
     if(!mcode && !still_running) {
diff --git a/lib/multi.c b/lib/multi.c
index 9b6801db7..d0ed64fad 100755
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -974,12 +974,12 @@ CURLMcode curl_multi_fdset(struct Curl_multi *multi,
 
 #define NUM_POLLS_ON_STACK 10
 
-CURLMcode Curl_multi_wait(struct Curl_multi *multi,
-                          struct curl_waitfd extra_fds[],
-                          unsigned int extra_nfds,
-                          int timeout_ms,
-                          int *ret,
-                          bool *gotsocket) /* if any socket was checked */
+static CURLMcode Curl_multi_wait(struct Curl_multi *multi,
+                                 struct curl_waitfd extra_fds[],
+                                 unsigned int extra_nfds,
+                                 int timeout_ms,
+                                 int *ret,
+                                 bool extrawait) /* when no socket, wait */
 {
   struct Curl_easy *data;
   curl_socket_t sockbunch[MAX_SOCKSPEREASYHANDLE];
@@ -993,9 +993,6 @@ CURLMcode Curl_multi_wait(struct Curl_multi *multi,
   struct pollfd a_few_on_stack[NUM_POLLS_ON_STACK];
   struct pollfd *ufds = &a_few_on_stack[0];
 
-  if(gotsocket)
-    *gotsocket = FALSE;
-
   if(!GOOD_MULTI_HANDLE(multi))
     return CURLM_BAD_HANDLE;
 
@@ -1124,9 +1121,19 @@ CURLMcode Curl_multi_wait(struct Curl_multi *multi,
     free(ufds);
   if(ret)
     *ret = retcode;
-  if(gotsocket && (extra_fds || curlfds))
+  if(!extrawait || extra_fds || curlfds)
     /* if any socket was checked */
-    *gotsocket = TRUE;
+    ;
+  else {
+    long sleep_ms = 0;
+
+    /* Avoid busy-looping when there's nothing particular to wait for */
+    if(!curl_multi_timeout(multi, &sleep_ms) && sleep_ms) {
+      if(sleep_ms > timeout_ms)
+        sleep_ms = timeout_ms;
+      Curl_wait_ms((int)sleep_ms);
+    }
+  }
 
   return CURLM_OK;
 }
@@ -1137,7 +1144,16 @@ CURLMcode curl_multi_wait(struct Curl_multi *multi,
                           int timeout_ms,
                           int *ret)
 {
-  return Curl_multi_wait(multi, extra_fds, extra_nfds, timeout_ms, ret, NULL);
+  return Curl_multi_wait(multi, extra_fds, extra_nfds, timeout_ms, ret, FALSE);
+}
+
+CURLMcode curl_multi_poll(struct Curl_multi *multi,
+                          struct curl_waitfd extra_fds[],
+                          unsigned int extra_nfds,
+                          int timeout_ms,
+                          int *ret)
+{
+  return Curl_multi_wait(multi, extra_fds, extra_nfds, timeout_ms, ret, TRUE);
 }
 
 /*
diff --git a/lib/multiif.h b/lib/multiif.h
index a64455867..a94bfb433 100644
--- a/lib/multiif.h
+++ b/lib/multiif.h
@@ -89,11 +89,4 @@ CURLMcode Curl_multi_add_perform(struct Curl_multi *multi,
                                  struct Curl_easy *data,
                                  struct connectdata *conn);
 
-CURLMcode Curl_multi_wait(struct Curl_multi *multi,
-                          struct curl_waitfd extra_fds[],
-                          unsigned int extra_nfds,
-                          int timeout_ms,
-                          int *ret,
-                          bool *gotsocket); /* if any socket was checked */
-
 #endif /* HEADER_CURL_MULTIIF_H */
diff --git a/src/tool_operate.c b/src/tool_operate.c
index 7cb2f01f9..946dc7cca 100644
--- a/src/tool_operate.c
+++ b/src/tool_operate.c
@@ -1904,23 +1904,6 @@ static CURLcode create_transfers(struct GlobalConfig *global,
   return result;
 }
 
-/* portable millisecond sleep */
-static void wait_ms(int ms)
-{
-#if defined(MSDOS)
-  delay(ms);
-#elif defined(WIN32)
-  Sleep(ms);
-#elif defined(HAVE_USLEEP)
-  usleep(1000 * ms);
-#else
-  struct timeval pending_tv;
-  pending_tv.tv_sec = ms / 1000;
-  pending_tv.tv_usec = (ms % 1000) * 1000;
-  (void)select(0, NULL, NULL, NULL, &pending_tv);
-#endif
-}
-
 static long all_added; /* number of easy handles currently added */
 
 static int add_parallel_transfers(struct GlobalConfig *global,
@@ -1971,30 +1954,9 @@ static CURLcode parallel_transfers(struct GlobalConfig *global,
     return result;
 
   while(!done && !mcode && still_running) {
-    int numfds;
-    struct timeval before = tvnow();
-    long delta;
-
-    mcode = curl_multi_wait(multi, NULL, 0, 1000, &numfds);
-    delta = tvdiff(tvnow(), before);
-
-    if(!mcode) {
-      if(!numfds && (delta < 30)) {
-        long sleep_ms;
-
-        /* If it returns without any file descriptor instantly, we need to
-           avoid busy-looping during periods where it has nothing particular
-           to wait for */
-        curl_multi_timeout(multi, &sleep_ms);
-        if(sleep_ms) {
-          if(sleep_ms > 1000)
-            sleep_ms = 1000;
-          wait_ms((int)sleep_ms);
-        }
-      }
-
+    mcode = curl_multi_poll(multi, NULL, 0, 1000, NULL);
+    if(!mcode)
       mcode = curl_multi_perform(multi, &still_running);
-    }
 
     progress_meter(global, &start, FALSE);
 
diff --git a/tests/data/test1135 b/tests/data/test1135
index 3591a543b..eca6860fb 100644
--- a/tests/data/test1135
+++ b/tests/data/test1135
@@ -91,6 +91,7 @@ CURL_EXTERN CURLMcode curl_multi_add_handle(CURLM *multi_handle,
 CURL_EXTERN CURLMcode curl_multi_remove_handle(CURLM *multi_handle,
 CURL_EXTERN CURLMcode curl_multi_fdset(CURLM *multi_handle,
 CURL_EXTERN CURLMcode curl_multi_wait(CURLM *multi_handle,
+CURL_EXTERN CURLMcode curl_multi_poll(CURLM *multi_handle,
 CURL_EXTERN CURLMcode curl_multi_perform(CURLM *multi_handle,
 CURL_EXTERN CURLMcode curl_multi_cleanup(CURLM *multi_handle);
 CURL_EXTERN CURLMsg *curl_multi_info_read(CURLM *multi_handle,