From 3e817a6abb55456024188237cdbbb77b9d39728e Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 2 Aug 2023 21:08:52 +0200 Subject: [PATCH] Revert the fix for GH-11498 People relied on manually waiting for children, but the fix for GH-11498 broke this. Fixing this in PHP is fundamentally incompatible with doing the wait loop in userland. This reverts to the old behaviour. --- ext/pcntl/pcntl.c | 55 ++++++------------------------------ ext/pcntl/tests/gh11498.phpt | 43 ---------------------------- 2 files changed, 8 insertions(+), 90 deletions(-) delete mode 100644 ext/pcntl/tests/gh11498.phpt diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c index 4eedbc574e5fc..ad04415d36c2d 100644 --- a/ext/pcntl/pcntl.c +++ b/ext/pcntl/pcntl.c @@ -1338,58 +1338,19 @@ static void pcntl_signal_handler(int signo, siginfo_t *siginfo, void *context) static void pcntl_signal_handler(int signo) #endif { - struct php_pcntl_pending_signal *psig_first = PCNTL_G(spares); - if (!psig_first) { + struct php_pcntl_pending_signal *psig = PCNTL_G(spares); + if (!psig) { /* oops, too many signals for us to track, so we'll forget about this one */ return; } + PCNTL_G(spares) = psig->next; - struct php_pcntl_pending_signal *psig = NULL; - - /* Standard signals may be merged into a single one. - * POSIX specifies that SIGCHLD has the si_pid field (https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html), - * so we'll handle the merging for that signal. - * See also: https://www.gnu.org/software/libc/manual/html_node/Merged-Signals.html */ - if (signo == SIGCHLD) { - /* Note: The first waitpid result is not necessarily the pid that was passed above! - * We therefore cannot avoid the first waitpid() call. */ - int status; - pid_t pid; - while (true) { - do { - errno = 0; - /* Although Linux specifies that WNOHANG will never result in EINTR, POSIX doesn't say so: - * https://pubs.opengroup.org/onlinepubs/9699919799/functions/waitpid.html */ - pid = waitpid(-1, &status, WNOHANG | WUNTRACED); - } while (pid <= 0 && errno == EINTR); - if (pid <= 0) { - if (UNEXPECTED(!psig)) { - /* The child might've been consumed by another thread and will be handled there. */ - return; - } - break; - } - - psig = psig ? psig->next : psig_first; - psig->signo = signo; - -#ifdef HAVE_STRUCT_SIGINFO_T - psig->siginfo = *siginfo; - psig->siginfo.si_pid = pid; -#endif - - if (UNEXPECTED(!psig->next)) { - break; - } - } - } else { - psig = psig_first; - psig->signo = signo; + psig->signo = signo; + psig->next = NULL; #ifdef HAVE_STRUCT_SIGINFO_T - psig->siginfo = *siginfo; + psig->siginfo = *siginfo; #endif - } PCNTL_G(spares) = psig->next; psig->next = NULL; @@ -1397,9 +1358,9 @@ static void pcntl_signal_handler(int signo) /* the head check is important, as the tick handler cannot atomically clear both * the head and tail */ if (PCNTL_G(head) && PCNTL_G(tail)) { - PCNTL_G(tail)->next = psig_first; + PCNTL_G(tail)->next = psig; } else { - PCNTL_G(head) = psig_first; + PCNTL_G(head) = psig; } PCNTL_G(tail) = psig; PCNTL_G(pending_signals) = 1; diff --git a/ext/pcntl/tests/gh11498.phpt b/ext/pcntl/tests/gh11498.phpt deleted file mode 100644 index 4a9f87a94d82e..0000000000000 --- a/ext/pcntl/tests/gh11498.phpt +++ /dev/null @@ -1,43 +0,0 @@ ---TEST-- -GH-11498 (SIGCHLD is not always returned from proc_open) ---EXTENSIONS-- -pcntl ---SKIPIF-- - ---FILE-- - 0) { - usleep(100_000); - $iters--; -} - -var_dump(empty($processes)); -?> ---EXPECT-- -SIGCHLD -SIGCHLD -SIGCHLD -SIGCHLD -SIGCHLD -SIGCHLD -bool(true)