From d11ea999be2e9f8df853672f68ff832c9469911c Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Tue, 7 Apr 2026 19:54:13 +0200 Subject: [PATCH v10 2/4] fe_utils: Simplify cancel logic in wait_on_slots This removes the SetCancelConn call in wait_on_slots. This call was confusing because it was only called for a single connection of all the connections that the select_loop would wait on. It can be made completely redundant by checking CancelRequested in select_loop itself. To do that reliably we need to wait for socket activity in a way that is race-free with respect to SIGINT: with plain select() a SIGINT arriving between the CancelRequested check and the start of the wait could leave us blocked until a socket happens to become readable, which for a stuck server might be never. On platforms that have pselect() we now use it, keeping SIGINT blocked around the CancelRequested check and letting pselect() unblock it atomically for the duration of the wait only. That way a SIGINT delivered in the race window stays pending and interrupts pselect() immediately. On platforms without pselect() (including Windows) we fall back to select() with a 1 second timeout and poll CancelRequested manually. --- configure | 2 +- configure.ac | 1 + meson.build | 1 + src/fe_utils/parallel_slot.c | 85 ++++++++++++++++++++++++------------ src/include/pg_config.h.in | 3 ++ 5 files changed, 64 insertions(+), 28 deletions(-) diff --git a/configure b/configure index 35b0b72f0a7..24859dad385 100755 --- a/configure +++ b/configure @@ -15888,7 +15888,7 @@ fi LIBS_including_readline="$LIBS" LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` -for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info explicit_memset getauxval getifaddrs getpeerucred inet_pton kqueue localeconv_l mbstowcs_l memset_explicit posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strsignal syncfs sync_file_range uselocale wcstombs_l +for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info explicit_memset getauxval getifaddrs getpeerucred inet_pton kqueue localeconv_l mbstowcs_l memset_explicit posix_fallocate ppoll pselect pthread_is_threaded_np setproctitle setproctitle_fast strsignal syncfs sync_file_range uselocale wcstombs_l do : as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" diff --git a/configure.ac b/configure.ac index 0e624fe36b9..5e11d7dafdc 100644 --- a/configure.ac +++ b/configure.ac @@ -1865,6 +1865,7 @@ AC_CHECK_FUNCS(m4_normalize([ memset_explicit posix_fallocate ppoll + pselect pthread_is_threaded_np setproctitle setproctitle_fast diff --git a/meson.build b/meson.build index d88a7a70308..8ef577132b9 100644 --- a/meson.build +++ b/meson.build @@ -3224,6 +3224,7 @@ func_checks = [ ['posix_fadvise'], ['posix_fallocate'], ['ppoll'], + ['pselect'], ['pthread_barrier_wait', {'dependencies': [thread_dep]}], ['pthread_is_threaded_np', {'dependencies': [thread_dep]}], ['sem_init', {'dependencies': [rt_dep, thread_dep], 'skip': sema_kind != 'unnamed_posix', 'define': false}], diff --git a/src/fe_utils/parallel_slot.c b/src/fe_utils/parallel_slot.c index fb9e6cc4ec1..d913e14c625 100644 --- a/src/fe_utils/parallel_slot.c +++ b/src/fe_utils/parallel_slot.c @@ -19,6 +19,10 @@ #include "postgres_fe.h" #include +#ifdef HAVE_PSELECT +#include +#include +#endif #include "common/logging.h" #include "fe_utils/cancel.h" @@ -82,27 +86,56 @@ select_loop(int maxFd, fd_set *workerset) int i; fd_set saveSet = *workerset; - if (CancelRequested) - return -1; +#ifdef HAVE_PSELECT + + /* + * On platforms that have it, we wait with pselect() rather than select(), + * so that we can react to a cancel request without polling for it. The + * problem with plain select() is a race: SIGINT can arrive after we check + * CancelRequested but before select() starts waiting, in which case + * select() would keep waiting until a socket happens to become readable + * (which for a stuck server might be never). We close that race by + * keeping SIGINT blocked around the CancelRequested check and having + * pselect() unblock it (via origmask) only for the duration of the wait: + * a SIGINT delivered in the race window stays pending and then interrupts + * pselect() immediately with EINTR, so we loop around and bail out. + */ + sigset_t origmask; + sigset_t blockmask; + + sigemptyset(&blockmask); + sigaddset(&blockmask, SIGINT); + pthread_sigmask(SIG_BLOCK, &blockmask, &origmask); +#endif for (;;) { + if (CancelRequested) + { + i = -1; + break; + } + + *workerset = saveSet; +#ifdef HAVE_PSELECT + /* NULL timeout: wait indefinitely, relying on SIGINT to wake us. */ + i = pselect(maxFd + 1, workerset, NULL, NULL, NULL, &origmask); +#else + /* - * On Windows, we need to check once in a while for cancel requests; - * on other platforms we rely on select() returning when interrupted. + * Without pselect() we can't wait race-free, so add a timeout of 1 + * second to the select() and poll CancelRequested manually as a + * fallback. This also covers Windows, where a cancel request is + * signalled by a separate console-handler thread rather than by + * interrupting the wait. */ - struct timeval *tvp; -#ifdef WIN32 - struct timeval tv = {0, 1000000}; + { + struct timeval tv = {0, 1000000}; - tvp = &tv; -#else - tvp = NULL; + i = select(maxFd + 1, workerset, NULL, NULL, &tv); + } #endif - *workerset = saveSet; - i = select(maxFd + 1, workerset, NULL, NULL, tvp); - #ifdef WIN32 if (i == SOCKET_ERROR) { @@ -114,14 +147,19 @@ select_loop(int maxFd, fd_set *workerset) #endif if (i < 0 && errno == EINTR) - continue; /* ignore this */ - if (i < 0 || CancelRequested) - return -1; /* but not this */ + continue; /* interrupted, re-check CancelRequested */ if (i == 0) - continue; /* timeout (Win32 only) */ - break; + continue; /* timeout (only reachable via select()) */ + break; /* ready sockets, or a hard error */ } +#ifdef HAVE_PSELECT + pthread_sigmask(SIG_SETMASK, &origmask, NULL); +#endif + + if (i < 0) + return -1; + return i; } @@ -197,8 +235,7 @@ wait_on_slots(ParallelSlotArray *sa) { int i; fd_set slotset; - int maxFd = 0; - PGconn *cancelconn = NULL; + int maxFd = -1; /* We must reconstruct the fd_set for each call to select_loop */ FD_ZERO(&slotset); @@ -219,10 +256,6 @@ wait_on_slots(ParallelSlotArray *sa) if (sock < 0) continue; - /* Keep track of the first valid connection we see. */ - if (cancelconn == NULL) - cancelconn = sa->slots[i].connection; - FD_SET(sock, &slotset); if (sock > maxFd) maxFd = sock; @@ -232,12 +265,10 @@ wait_on_slots(ParallelSlotArray *sa) * If we get this far with no valid connections, processing cannot * continue. */ - if (cancelconn == NULL) + if (maxFd < 0) return false; - SetCancelConn(cancelconn); i = select_loop(maxFd, &slotset); - ResetCancelConn(); /* failure? */ if (i < 0) diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 4f8113c144b..3c1e0d14a65 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -309,6 +309,9 @@ /* Define to 1 if you have the `ppoll' function. */ #undef HAVE_PPOLL +/* Define to 1 if you have the `pselect' function. */ +#undef HAVE_PSELECT + /* Define if you have POSIX threads libraries and header files. */ #undef HAVE_PTHREAD -- 2.54.0