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.

Author: nielsdos

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)
 

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;

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)