From: Daniel Stenberg <daniel@haxx.se>
Date: Mon, 30 Sep 2024 21:43:58 +0000 (+0200)
Subject: select: use poll() if existing, avoid poll() with no sockets
X-Git-Tag: curl-8_11_0~280
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c72cefea0fadaf4114a0036c86005ee5739ec30a;p=thirdparty%2Fcurl.git

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 e6e64bb7d5..5e8f03f1d0 100644
--- 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 57b2b40bd4..1cedad3fd7 100644
--- 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 15964dd860..af8d1b030e 100644
--- 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 09db30cc40..e5998a1062 100644
--- 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 e56aca15cf..1270e108dd 100644
--- 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 ef7a7a6933..dd806da9bc 100644
--- 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 dae736b019..c1779ad1e5 100644
--- 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 851d50977f..fd4aac336b 100644
--- 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 31b5f01c92..b1d14a177a 100644
--- 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 4cde4b1e47..289692c3f9 100644
--- 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 d05cd16ab1..6e1baee7ed 100644
--- 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 91d35513c8..96f44fe0be 100644
--- 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;