From f39b5139161f3e31fcb7e6bd2e3b7c89dc4ae53c 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] 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.

Closes GH-11509.
---
 NEWS                         |  4 +++
 ext/pcntl/pcntl.c            | 55 ++++++++++++++++++++++++++++++++----
 ext/pcntl/tests/gh11498.phpt | 35 +++++++++++++++++++++++
 3 files changed, 88 insertions(+), 6 deletions(-)
 create mode 100644 ext/pcntl/tests/gh11498.phpt

diff --git a/NEWS b/NEWS
index d05e8e14085b1..a1815f17de25d 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,10 @@ PHP                                                                        NEWS
 - Date:
   . Fixed bug GH-11368 (Date modify returns invalid datetime). (Derick)
 
+- PCNTL:
+  . Fixed bug GH-11498 (SIGCHLD is not always returned from proc_open).
+    (nielsdos)
+
 - PCRE:
   . Mangle PCRE regex cache key with JIT option. (mvorisek)
 
diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c
index c691d32239e36..df0f23ca68f43 100644
--- a/ext/pcntl/pcntl.c
+++ b/ext/pcntl/pcntl.c
@@ -1345,21 +1345,64 @@ 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.
+	 * 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(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.si_pid = pid;
+#endif
+
+			if (EXPECTED(psig->next)) {
+				psig = psig->next;
+			} else {
+				break;
+			}
+		}
+	} else {
+single_signal:;
+		psig->signo = signo;
 
 #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;
+		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)