From 24187672ff5a7976f763dd055ae83f51025952db Mon Sep 17 00:00:00 2001
From: nielsdos <7771979+nielsdos@users.noreply.github.com>
Date: Fri, 23 Jun 2023 00:04:07 +0200
Subject: [PATCH 1/2] Fix GH-11498: SIGCHLD is not always returned from
 proc_open

Linux, and maybe other unixes, may merge multiple standard signals into
a single one. This causes issues when keeping track of process IDs.
Solve this by manually checking which children are dead using waitpid().

Test case is based on taka-oyama's test code.
---
 ext/pcntl/pcntl.c            | 52 +++++++++++++++++++++++++++++++-----
 ext/pcntl/tests/gh11498.phpt | 35 ++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 6 deletions(-)
 create mode 100644 ext/pcntl/tests/gh11498.phpt

diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c
index c691d32239e36..5f3d30b5257bb 100644
--- a/ext/pcntl/pcntl.c
+++ b/ext/pcntl/pcntl.c
@@ -1345,21 +1345,61 @@ static void pcntl_signal_handler(int signo)
 		/* oops, too many signals for us to track, so we'll forget about this one */
 		return;
 	}
-	PCNTL_G(spares) = psig->next;
 
-	psig->signo = signo;
-	psig->next = NULL;
+	struct php_pcntl_pending_signal *psig_first = psig;
+
+	/* 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. */
+	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;
+		do {
+			do {
+				errno = 0;
+				pid = waitpid(WAIT_ANY, &status, WNOHANG);
+			} while (pid <= 0 && errno == EINTR);
+			if (pid <= 0) {
+				if (UNEXPECTED(psig == psig_first)) {
+					/* Don't handle multiple, revert back to the single signal handling. */
+					goto single_signal;
+				}
+				break;
+			}
+
+			psig->signo = signo;
 
 #ifdef HAVE_STRUCT_SIGINFO_T
-	psig->siginfo = *siginfo;
+			psig->siginfo = *siginfo;
+			psig->siginfo.si_pid = pid;
 #endif
 
+			if (EXPECTED(psig->next)) {
+				psig = psig->next;
+			} else {
+				break;
+			}
+		} while (pid);
+	} else {
+single_signal:;
+		psig->signo = signo;
+
+#ifdef HAVE_STRUCT_SIGINFO_T
+		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;
+		PCNTL_G(tail)->next = psig_first;
 	} else {
-		PCNTL_G(head) = psig;
+		PCNTL_G(head) = psig_first;
 	}
 	PCNTL_G(tail) = psig;
 	PCNTL_G(pending_signals) = 1;
diff --git a/ext/pcntl/tests/gh11498.phpt b/ext/pcntl/tests/gh11498.phpt
new file mode 100644
index 0000000000000..f6983c6f8d303
--- /dev/null
+++ b/ext/pcntl/tests/gh11498.phpt
@@ -0,0 +1,35 @@
+--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) {
+    unset($processes[$info['pid']]);
+}, false);
+
+foreach (range(0, 5) as $i) {
+    $process = proc_open('echo $$ > /dev/null', [], $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--
+bool(true)

From b438b1c4c849d03c584fa53c2f25013dd5f5acd9 Mon Sep 17 00:00:00 2001
From: nielsdos <7771979+nielsdos@users.noreply.github.com>
Date: Fri, 23 Jun 2023 17:55:23 +0200
Subject: [PATCH 2/2] Remove always-true condition

---
 ext/pcntl/pcntl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c
index 5f3d30b5257bb..c1aca155492a1 100644
--- a/ext/pcntl/pcntl.c
+++ b/ext/pcntl/pcntl.c
@@ -1356,7 +1356,7 @@ static void pcntl_signal_handler(int signo)
 		 *       We therefore cannot avoid the first waitpid() call. */
 		int status;
 		pid_t pid;
-		do {
+		while (true) {
 			do {
 				errno = 0;
 				pid = waitpid(WAIT_ANY, &status, WNOHANG);
@@ -1381,7 +1381,7 @@ static void pcntl_signal_handler(int signo)
 			} else {
 				break;
 			}
-		} while (pid);
+		}
 	} else {
 single_signal:;
 		psig->signo = signo;