From 02346abc32a3995299fa9c2f35b9f0a1d091b045 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 29 Jul 2019 13:41:00 +0200 Subject: curl_multi_poll: a sister to curl_multi_wait() that waits more 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. Closes #4163 --- docs/libcurl/Makefile.inc | 1 + docs/libcurl/curl_multi_poll.3 | 110 +++++++++++++++++++++++++++++++++++++++++ include/curl/multi.h | 16 +++++- lib/Makefile.am | 2 +- lib/easy.c | 22 ++------- lib/multi.c | 40 ++++++++++----- lib/multiif.h | 7 --- src/tool_operate.c | 42 +--------------- tests/data/test1135 | 1 + 9 files changed, 161 insertions(+), 80 deletions(-) create mode 100644 docs/libcurl/curl_multi_poll.3 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, , 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 + +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, , et al. + * Copyright (C) 1998 - 2019, Daniel Stenberg, , 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 839751edf..5d41c2be3 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 6e16eafe0..e5c483c56 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 57b6145eb..0755a7cd2 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, -- cgit v1.2.3