select: use poll() if existing, avoid poll() with no sockets

poll() on macOS 10.12 was deemed broken in 2016 when we discovered that
it misbehaves when provided with no sockets to wait for. The
HAVE_POLL_FINE is used to mark a poll() implementation that behaves
correctly: it *should* still wait the timeout time.

curl has therefore opted to use select() on Apple operating systems ever
since. To avoid the risk that this or other breakage cause problems.

However, using select() internally is also bad because it suffers from
problems when using file descriptors beyond 1024.

This change makes poll() used if it is present, but if there is no
sockets to wait for it avoids using poll() and instead falls back to
select() - but without any sockets to wait for there is no 1024 problem.

This removes all previous special-handling involving HAVE_POLL_FINE.

ref: https://daniel.haxx.se/blog/2016/10/11/poll-on-mac-10-12-is-broken/

Closes #15096

diff --git a/CMake/OtherTests.cmake b/CMake/OtherTests.cmake
index e6e64bb7d5fb860201dc242b459296866b56ced5..5e8f03f1d04e02844265b47fb169d9c05732355e 100644 (file)
--- a/CMake/OtherTests.cmake
+++ b/CMake/OtherTests.cmake
@@ -76,35 +76,6 @@ check_c_source_compiles("${_source_epilogue}
 
 unset(CMAKE_TRY_COMPILE_TARGET_TYPE)
 
-if(NOT APPLE)
-  set(_source_epilogue "#undef inline")
-  add_header_include(HAVE_SYS_POLL_H "sys/poll.h")
-  add_header_include(HAVE_POLL_H "poll.h")
-  if(NOT CMAKE_CROSSCOMPILING)
-    check_c_source_runs("${_source_epilogue}
-      #include <stdlib.h>
-      int main(void)
-      {
-        if(0 != poll(0, 0, 10)) {
-          return 1; /* fail */
-        }
-        return 0;
-      }" HAVE_POLL_FINE)
-  elseif(UNIX)
-    check_c_source_compiles("${_source_epilogue}
-      #include <stdlib.h>
-      int main(void)
-      {
-        #if defined(__BIONIC__) || \
-          defined(_POSIX_C_SOURCE) && _POSIX_C_SOURCE >= 200112L
-          (void)poll(0, 0, 0);
-        #else
-          #error force compilation error
-        #endif
-      }" HAVE_POLL_FINE)
-  endif()
-endif()
-
 # Detect HAVE_GETADDRINFO_THREADSAFE
 
 if(WIN32)

diff --git a/CMake/Platforms/WindowsCache.cmake b/CMake/Platforms/WindowsCache.cmake
index 57b2b40bd4387c4e0f7f6e94773bb6d551294991..1cedad3fd79d93cf1e67753fd0a211283b72b485 100644 (file)
--- a/CMake/Platforms/WindowsCache.cmake
+++ b/CMake/Platforms/WindowsCache.cmake
@@ -128,7 +128,7 @@ set(HAVE_NETINET_UDP_H 0)
 set(HAVE_NET_IF_H 0)
 set(HAVE_IOCTL_SIOCGIFADDR 0)
 set(HAVE_POLL_H 0)
-set(HAVE_POLL_FINE 0)
+set(HAVE_POLL 0)
 set(HAVE_PWD_H 0)
 set(HAVE_SYS_EVENTFD_H 0)
 set(HAVE_SYS_FILIO_H 0)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 15964dd860f1c36cdf018719d277fcb9212966ea..af8d1b030ec3fd19f9d7389e0cdf70e2a1143f5e 100644 (file)
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1466,6 +1466,7 @@ endif()
 check_symbol_exists("fnmatch"         "${CURL_INCLUDES};fnmatch.h" HAVE_FNMATCH)
 check_symbol_exists("basename"        "${CURL_INCLUDES};string.h" HAVE_BASENAME)
 check_symbol_exists("opendir"         "${CURL_INCLUDES};dirent.h" HAVE_OPENDIR)
+check_symbol_exists("poll"            "${CURL_INCLUDES}" HAVE_POLL)
 check_symbol_exists("socket"          "${CURL_INCLUDES}" HAVE_SOCKET)
 check_symbol_exists("sched_yield"     "${CURL_INCLUDES};sched.h" HAVE_SCHED_YIELD)
 check_symbol_exists("socketpair"      "${CURL_INCLUDES}" HAVE_SOCKETPAIR)

diff --git a/configure.ac b/configure.ac
index 09db30cc401fae465c01c428aea69a5cd8db2275..e5998a106283444fd12a3dbeab8efc88448f49c9 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -4108,6 +4108,7 @@ AC_CHECK_FUNCS([\
   if_nametoindex \
   mach_absolute_time \
   pipe \
+  poll \
   sched_yield \
   sendmsg \
   sendmmsg \

diff --git a/lib/config-plan9.h b/lib/config-plan9.h
index e56aca15cff184e6377aaee86d0079050d0d6788..1270e108dd25749b3e8a0aadeefc28380749c332 100644 (file)
--- a/lib/config-plan9.h
+++ b/lib/config-plan9.h
@@ -103,7 +103,7 @@
 #define USE_OPENSSL 1
 
 #define HAVE_PIPE 1
-#define HAVE_POLL_FINE 1
+#define HAVE_POLL 1
 #define HAVE_POLL_H 1
 #define HAVE_PTHREAD_H 1
 #define HAVE_SETLOCALE 1

diff --git a/lib/curl_config.h.cmake b/lib/curl_config.h.cmake
index ef7a7a69338dc2cc5b6d509ddb60c968c2ff512d..dd806da9bca3bbbdf7b5e2d19f36453fb9868ee1 100644 (file)
--- a/lib/curl_config.h.cmake
+++ b/lib/curl_config.h.cmake
@@ -430,8 +430,8 @@
 /* Define to 1 if you have the `eventfd' function. */
 #cmakedefine HAVE_EVENTFD 1
 
-/* If you have a fine poll */
-#cmakedefine HAVE_POLL_FINE 1
+/* If you have poll */
+#cmakedefine HAVE_POLL 1
 
 /* Define to 1 if you have the <poll.h> header file. */
 #cmakedefine HAVE_POLL_H 1

diff --git a/lib/select.c b/lib/select.c
index dae736b019ab796bac8f0d43c842a1912516acfe..c1779ad1e51188c0c45af9165dc78adf562048ac 100644 (file)
--- a/lib/select.c
+++ b/lib/select.c
@@ -24,6 +24,10 @@
 
 #include "curl_setup.h"
 
+#if !defined(HAVE_SELECT) && !defined(HAVE_POLL)
+#error "We cannot compile without select() or poll() support."
+#endif
+
 #include <limits.h>
 
 #ifdef HAVE_SYS_SELECT_H
@@ -32,10 +36,6 @@
 #include <unistd.h>
 #endif
 
-#if !defined(HAVE_SELECT) && !defined(HAVE_POLL_FINE)
-#error "We cannot compile without select() or poll() support."
-#endif
-
 #ifdef MSDOS
 #include <dos.h>  /* delay() */
 #endif
@@ -53,16 +53,15 @@
 #include "memdebug.h"
 
 /*
- * Internal function used for waiting a specific amount of ms
- * in Curl_socket_check() and Curl_poll() when no file descriptor
- * is provided to wait on, just being used to delay execution.
- * Winsock select() and poll() timeout mechanisms need a valid
- * socket descriptor in a not null file descriptor set to work.
- * Waiting indefinitely with this function is not allowed, a
- * zero or negative timeout value will return immediately.
- * Timeout resolution, accuracy, as well as maximum supported
- * value is system dependent, neither factor is a critical issue
- * for the intended use of this function in the library.
+ * Internal function used for waiting a specific amount of ms in
+ * Curl_socket_check() and Curl_poll() when no file descriptor is provided to
+ * wait on, just being used to delay execution. Winsock select() and poll()
+ * timeout mechanisms need a valid socket descriptor in a not null file
+ * descriptor set to work. Waiting indefinitely with this function is not
+ * allowed, a zero or negative timeout value will return immediately. Timeout
+ * resolution, accuracy, as well as maximum supported value is system
+ * dependent, neither factor is a critical issue for the intended use of this
+ * function in the library.
  *
  * Return values:
  *   -1 = system call error, or invalid timeout value
@@ -89,20 +88,13 @@ int Curl_wait_ms(timediff_t timeout_ms)
 #endif
   Sleep((ULONG)timeout_ms);
 #else
-#if defined(HAVE_POLL_FINE)
-  /* prevent overflow, timeout_ms is typecast to int. */
-#if TIMEDIFF_T_MAX > INT_MAX
-  if(timeout_ms > INT_MAX)
-    timeout_ms = INT_MAX;
-#endif
-  r = poll(NULL, 0, (int)timeout_ms);
-#else
+  /* avoid using poll() for this since it behaves incorrectly with no sockets
+     on Apple operating systems */
   {
     struct timeval pending_tv;
     r = select(0, NULL, NULL, NULL, curlx_mstotv(&pending_tv, timeout_ms));
   }
-#endif /* HAVE_POLL_FINE */
-#endif /* USE_WINSOCK */
+#endif /* _WIN32 */
   if(r) {
     if((r == -1) && (SOCKERRNO == EINTR))
       /* make EINTR from select or poll not a "lethal" error */
@@ -113,12 +105,12 @@ int Curl_wait_ms(timediff_t timeout_ms)
   return r;
 }
 
-#ifndef HAVE_POLL_FINE
+#ifndef HAVE_POLL
 /*
- * This is a wrapper around select() to aid in Windows compatibility.
- * A negative timeout value makes this function wait indefinitely,
- * unless no valid file descriptor is given, when this happens the
- * negative timeout is ignored and the function times out immediately.
+ * This is a wrapper around select() to aid in Windows compatibility. A
+ * negative timeout value makes this function wait indefinitely, unless no
+ * valid file descriptor is given, when this happens the negative timeout is
+ * ignored and the function times out immediately.
  *
  * Return values:
  *   -1 = system call error or fd >= FD_SETSIZE
@@ -172,13 +164,13 @@ static int our_select(curl_socket_t maxfd,   /* highest socket number */
 
 /*
  * Wait for read or write events on a set of file descriptors. It uses poll()
- * when a fine poll() is available, in order to avoid limits with FD_SETSIZE,
+ * when poll() is available, in order to avoid limits with FD_SETSIZE,
  * otherwise select() is used. An error is returned if select() is being used
  * and a file descriptor is too large for FD_SETSIZE.
  *
- * A negative timeout value makes this function wait indefinitely,
- * unless no valid file descriptor is given, when this happens the
- * negative timeout is ignored and the function times out immediately.
+ * A negative timeout value makes this function wait indefinitely, unless no
+ * valid file descriptor is given, when this happens the negative timeout is
+ * ignored and the function times out immediately.
  *
  * Return values:
  *   -1 = system call error or fd >= FD_SETSIZE
@@ -275,7 +267,7 @@ int Curl_socket_check(curl_socket_t readfd0, /* two sockets to read from */
  */
 int Curl_poll(struct pollfd ufds[], unsigned int nfds, timediff_t timeout_ms)
 {
-#ifdef HAVE_POLL_FINE
+#ifdef HAVE_POLL
   int pending_ms;
 #else
   fd_set fds_read;
@@ -305,7 +297,7 @@ int Curl_poll(struct pollfd ufds[], unsigned int nfds, timediff_t timeout_ms)
      when function is called with a zero timeout or a negative timeout
      value indicating a blocking call should be performed. */
 
-#ifdef HAVE_POLL_FINE
+#ifdef HAVE_POLL
 
   /* prevent overflow, timeout_ms is typecast to int. */
 #if TIMEDIFF_T_MAX > INT_MAX
@@ -335,7 +327,7 @@ int Curl_poll(struct pollfd ufds[], unsigned int nfds, timediff_t timeout_ms)
       ufds[i].revents |= POLLIN|POLLOUT;
   }
 
-#else  /* HAVE_POLL_FINE */
+#else  /* HAVE_POLL */
 
   FD_ZERO(&fds_read);
   FD_ZERO(&fds_write);
@@ -401,7 +393,7 @@ int Curl_poll(struct pollfd ufds[], unsigned int nfds, timediff_t timeout_ms)
       r++;
   }
 
-#endif  /* HAVE_POLL_FINE */
+#endif  /* HAVE_POLL */
 
   return r;
 }

diff --git a/m4/curl-functions.m4 b/m4/curl-functions.m4
index 851d50977fc5bf2d9b7ddd6c74835060c00afca3..fd4aac336b6244388c83b6ce72a23b60a3b14d59 100644 (file)
--- a/m4/curl-functions.m4
+++ b/m4/curl-functions.m4
@@ -3407,21 +3407,6 @@ AC_DEFUN([CURL_CHECK_FUNC_POLL], [
   tst_links_poll="unknown"
   tst_proto_poll="unknown"
   tst_compi_poll="unknown"
-  tst_works_poll="unknown"
-  tst_allow_poll="unknown"
-  #
-  case $host in
-    *-apple-*|*-*-interix*)
-      dnl poll() does not work on these platforms
-      dnl Interix: "does provide poll(), but the implementing developer must
-      dnl have been in a bad mood, because poll() only works on the /proc
-      dnl filesystem here"
-      dnl macOS: poll() first didn't exist, then was broken until fixed in 10.9
-      dnl only to break again in 10.12.
-      curl_disallow_poll="yes"
-      tst_compi_poll="no"
-      ;;
-  esac
   #
   AC_MSG_CHECKING([if poll can be linked])
   AC_LINK_IFELSE([
@@ -3464,82 +3449,13 @@ AC_DEFUN([CURL_CHECK_FUNC_POLL], [
     ],[
       AC_MSG_RESULT([yes])
       tst_compi_poll="yes"
+      AC_DEFINE_UNQUOTED(HAVE_POLL, 1, [If you have poll])
     ],[
       AC_MSG_RESULT([no])
       tst_compi_poll="no"
     ])
   fi
   #
-  dnl only do runtime verification when not cross-compiling
-  if test "$tst_compi_poll" = "yes"; then
-    if test "x$cross_compiling" != "xyes"; then
-      AC_MSG_CHECKING([if poll seems to work])
-      CURL_RUN_IFELSE([
-        AC_LANG_PROGRAM([[
-          $curl_includes_stdlib
-          $curl_includes_poll
-        ]],[[
-          /* detect the original poll() breakage */
-          if(0 != poll(0, 0, 10)) {
-            return 1; /* fail */
-          }
-        ]])
-      ],[
-        AC_MSG_RESULT([yes])
-        tst_works_poll="yes"
-      ],[
-        AC_MSG_RESULT([no])
-        tst_works_poll="no"
-      ])
-    else
-      AC_MSG_CHECKING([if native poll seems to be supported])
-      AC_COMPILE_IFELSE([
-        AC_LANG_PROGRAM([[
-          $curl_includes_stdlib
-        ]],[[
-          #if defined(__BIONIC__) || \
-            (defined(_POSIX_C_SOURCE) && _POSIX_C_SOURCE >= 200112L)
-            return 0;
-          #else
-            #error force compilation error
-          #endif
-        ]])
-      ],[
-        AC_MSG_RESULT([yes])
-        tst_works_poll="yes"
-      ],[
-        AC_MSG_RESULT([no])
-        tst_works_poll="no"
-      ])
-    fi
-  fi
-  #
-  if test "$tst_compi_poll" = "yes" &&
-    test "$tst_works_poll" != "no"; then
-    AC_MSG_CHECKING([if poll usage allowed])
-    if test "x$curl_disallow_poll" != "xyes"; then
-      AC_MSG_RESULT([yes])
-      tst_allow_poll="yes"
-    else
-      AC_MSG_RESULT([no])
-      tst_allow_poll="no"
-    fi
-  fi
-  #
-  AC_MSG_CHECKING([if poll might be used])
-  if test "$tst_links_poll" = "yes" &&
-     test "$tst_proto_poll" = "yes" &&
-     test "$tst_compi_poll" = "yes" &&
-     test "$tst_allow_poll" = "yes" &&
-     test "$tst_works_poll" != "no"; then
-    AC_MSG_RESULT([yes])
-    AC_DEFINE_UNQUOTED(HAVE_POLL_FINE, 1,
-      [If you have a fine poll])
-    curl_cv_func_poll="yes"
-  else
-    AC_MSG_RESULT([no])
-    curl_cv_func_poll="no"
-  fi
 ])
 
 

diff --git a/src/tool_sleep.c b/src/tool_sleep.c
index 31b5f01c929daf716c302aa9e78280e8ca096b8b..b1d14a177a242f4032783e6748046a3e79ea061c 100644 (file)
--- a/src/tool_sleep.c
+++ b/src/tool_sleep.c
@@ -49,8 +49,6 @@ void tool_go_sleep(long ms)
   delay(ms);
 #elif defined(_WIN32)
   Sleep((DWORD)ms);
-#elif defined(HAVE_POLL_FINE)
-  (void)poll((void *)0, 0, (int)ms);
 #else
   struct timeval timeout;
   timeout.tv_sec = ms / 1000L;

diff --git a/tests/libtest/lib518.c b/tests/libtest/lib518.c
index 4cde4b1e47f1d92550d4e6f0f93001b7ee6f9ab2..289692c3f9ce572e6fc98ef53ac0f239536590c7 100644 (file)
--- a/tests/libtest/lib518.c
+++ b/tests/libtest/lib518.c
@@ -368,7 +368,7 @@ static int test_rlimit(int keep_open)
   rlim2str(strbuff, sizeof(strbuff), num_open.rlim_max);
   fprintf(stderr, "%s file descriptors open\n", strbuff);
 
-#if !defined(HAVE_POLL_FINE) && !defined(USE_WINSOCK)
+#if !defined(HAVE_POLL) && !defined(USE_WINSOCK)
 
   /*
    * when using select() instead of poll() we cannot test

diff --git a/tests/libtest/lib537.c b/tests/libtest/lib537.c
index d05cd16ab11f17d210598d5aff6efe4f79ccbe0d..6e1baee7ed4b5ab5a7b367c830c4c7e87539b925 100644 (file)
--- a/tests/libtest/lib537.c
+++ b/tests/libtest/lib537.c
@@ -34,9 +34,7 @@
 #include "warnless.h"
 #include "memdebug.h"
 
-#if !defined(HAVE_POLL_FINE)    && \
-    !defined(USE_WINSOCK)       && \
-    !defined(FD_SETSIZE)
+#if !defined(HAVE_POLL) && !defined(USE_WINSOCK) && !defined(FD_SETSIZE)
 #error "this test requires FD_SETSIZE"
 #endif
 
@@ -381,7 +379,7 @@ static int test_rlimit(int keep_open)
   rlim2str(strbuff, sizeof(strbuff), num_open.rlim_max);
   fprintf(stderr, "%s file descriptors open\n", strbuff);
 
-#if !defined(HAVE_POLL_FINE) && !defined(USE_WINSOCK)
+#if !defined(HAVE_POLL) && !defined(USE_WINSOCK)
 
   /*
    * when using select() instead of poll() we cannot test

diff --git a/tests/server/util.c b/tests/server/util.c
index 91d35513c8f609a8d5dade94aa7403c276b7f167..96f44fe0be3136d6b673bf575ad6fcb7974b3823 100644 (file)
--- a/tests/server/util.c
+++ b/tests/server/util.c
@@ -229,7 +229,7 @@ FILE *test2fopen(long testno, const char *logdir)
 int wait_ms(int timeout_ms)
 {
 #if !defined(MSDOS) && !defined(USE_WINSOCK)
-#ifndef HAVE_POLL_FINE
+#ifndef HAVE_POLL
   struct timeval pending_tv;
 #endif
   struct timeval initial_tv;
@@ -252,13 +252,13 @@ int wait_ms(int timeout_ms)
   initial_tv = tvnow();
   do {
     int error;
-#if defined(HAVE_POLL_FINE)
+#ifdef HAVE_POLL
     r = poll(NULL, 0, pending_ms);
 #else
     pending_tv.tv_sec = pending_ms / 1000;
     pending_tv.tv_usec = (pending_ms % 1000) * 1000;
     r = select(0, NULL, NULL, NULL, &pending_tv);
-#endif /* HAVE_POLL_FINE */
+#endif /* HAVE_POLL */
     if(r != -1)
       break;
     error = errno;