From 9ae65a0bc39626780868ea2086817c7a19bab25a 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 1/2] 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            | 58 +++++-------------------------------
 ext/pcntl/tests/gh11498.phpt | 43 --------------------------
 2 files changed, 8 insertions(+), 93 deletions(-)
 delete mode 100644 ext/pcntl/tests/gh11498.phpt

diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c
index 4eedbc574e5fc..3ef19c96efac8 100644
--- a/ext/pcntl/pcntl.c
+++ b/ext/pcntl/pcntl.c
@@ -1338,68 +1338,26 @@ 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;
 
 	/* 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--
-<?php
-if (PHP_OS != 'Linux') {
-    die('skip Linux only');
-}
-?>
---FILE--
-<?php
-$processes = [];
-
-pcntl_async_signals(true);
-pcntl_signal(SIGCHLD, function($sig, $info) use (&$processes) {
-    echo "SIGCHLD\n";
-    unset($processes[$info['pid']]);
-}, false);
-
-for ($i = 0; $i <= 5; $i++) {
-    // Sleeping ensures we get to add the process to the list before the signal is invoked.
-    $process = proc_open('sleep 1', [], $pipes);
-    $pid = proc_get_status($process)['pid'];
-    $processes[$pid] = $process;
-}
-
-$iters = 50;
-while (!empty($processes) && $iters > 0) {
-    usleep(100_000);
-    $iters--;
-}
-
-var_dump(empty($processes));
-?>
---EXPECT--
-SIGCHLD
-SIGCHLD
-SIGCHLD
-SIGCHLD
-SIGCHLD
-SIGCHLD
-bool(true)

From 8d5db29a1dd7770f61c35c89315bc3ef89525762 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Thu, 3 Aug 2023 20:45:32 +0200
Subject: [PATCH 2/2] Add test

---
 .../tests/waiting_on_sigchild_pcntl_wait.phpt | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 ext/pcntl/tests/waiting_on_sigchild_pcntl_wait.phpt

diff --git a/ext/pcntl/tests/waiting_on_sigchild_pcntl_wait.phpt b/ext/pcntl/tests/waiting_on_sigchild_pcntl_wait.phpt
new file mode 100644
index 0000000000000..482888cfadd97
--- /dev/null
+++ b/ext/pcntl/tests/waiting_on_sigchild_pcntl_wait.phpt
@@ -0,0 +1,45 @@
+--TEST--
+Waiting on SIGCHLD with a pcntl_wait() loop
+--EXTENSIONS--
+pcntl
+--SKIPIF--
+<?php
+if (PHP_OS != 'Linux') {
+    die('skip Linux only');
+}
+?>
+--FILE--
+<?php
+$processes = [];
+
+pcntl_async_signals(true);
+pcntl_signal(SIGCHLD, function($sig, $info) use (&$processes) {
+    while (($pid = pcntl_wait($status, WUNTRACED | WNOHANG)) > 0) {
+        echo "SIGCHLD\n";
+        unset($processes[$pid]);
+    }
+}, false);
+
+for ($i = 0; $i <= 5; $i++) {
+    // Sleeping ensures we get to add the process to the list before the signal is invoked.
+    $process = proc_open('sleep 1', [], $pipes);
+    $pid = proc_get_status($process)['pid'];
+    $processes[$pid] = $process;
+}
+
+$iters = 50;
+while (!empty($processes) && $iters > 0) {
+    usleep(100_000);
+    $iters--;
+}
+
+var_dump(empty($processes));
+?>
+--EXPECT--
+SIGCHLD
+SIGCHLD
+SIGCHLD
+SIGCHLD
+SIGCHLD
+SIGCHLD
+bool(true)