Skip to content

Commit 2418767

Browse files
committed
Fix phpGH-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.
1 parent 039dd0b commit 2418767

File tree

2 files changed

+81
-6
lines changed

2 files changed

+81
-6
lines changed

ext/pcntl/pcntl.c

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,21 +1345,61 @@ static void pcntl_signal_handler(int signo)
13451345
/* oops, too many signals for us to track, so we'll forget about this one */
13461346
return;
13471347
}
1348-
PCNTL_G(spares) = psig->next;
13491348

1350-
psig->signo = signo;
1351-
psig->next = NULL;
1349+
struct php_pcntl_pending_signal *psig_first = psig;
1350+
1351+
/* Standard signals may be merged into a single one.
1352+
* POSIX specifies that SIGCHLD has the si_pid field (https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html),
1353+
* so we'll handle the merging for that signal. */
1354+
if (signo == SIGCHLD) {
1355+
/* Note: The first waitpid result is not necessarily the pid that was passed above!
1356+
* We therefore cannot avoid the first waitpid() call. */
1357+
int status;
1358+
pid_t pid;
1359+
do {
1360+
do {
1361+
errno = 0;
1362+
pid = waitpid(WAIT_ANY, &status, WNOHANG);
1363+
} while (pid <= 0 && errno == EINTR);
1364+
if (pid <= 0) {
1365+
if (UNEXPECTED(psig == psig_first)) {
1366+
/* Don't handle multiple, revert back to the single signal handling. */
1367+
goto single_signal;
1368+
}
1369+
break;
1370+
}
1371+
1372+
psig->signo = signo;
13521373

13531374
#ifdef HAVE_STRUCT_SIGINFO_T
1354-
psig->siginfo = *siginfo;
1375+
psig->siginfo = *siginfo;
1376+
psig->siginfo.si_pid = pid;
13551377
#endif
13561378

1379+
if (EXPECTED(psig->next)) {
1380+
psig = psig->next;
1381+
} else {
1382+
break;
1383+
}
1384+
} while (pid);
1385+
} else {
1386+
single_signal:;
1387+
psig->signo = signo;
1388+
1389+
#ifdef HAVE_STRUCT_SIGINFO_T
1390+
psig->siginfo = *siginfo;
1391+
#endif
1392+
}
1393+
1394+
PCNTL_G(spares) = psig->next;
1395+
psig->next = NULL;
1396+
13571397
/* the head check is important, as the tick handler cannot atomically clear both
13581398
* the head and tail */
13591399
if (PCNTL_G(head) && PCNTL_G(tail)) {
1360-
PCNTL_G(tail)->next = psig;
1400+
PCNTL_G(tail)->next = psig_first;
13611401
} else {
1362-
PCNTL_G(head) = psig;
1402+
PCNTL_G(head) = psig_first;
13631403
}
13641404
PCNTL_G(tail) = psig;
13651405
PCNTL_G(pending_signals) = 1;

ext/pcntl/tests/gh11498.phpt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
GH-11498 (SIGCHLD is not always returned from proc_open)
3+
--EXTENSIONS--
4+
pcntl
5+
--SKIPIF--
6+
<?php
7+
if (PHP_OS != 'Linux') {
8+
die('skip Linux only');
9+
}
10+
?>
11+
--FILE--
12+
<?php
13+
$processes = [];
14+
15+
pcntl_async_signals(true);
16+
pcntl_signal(SIGCHLD, function($sig, $info) use (&$processes) {
17+
unset($processes[$info['pid']]);
18+
}, false);
19+
20+
foreach (range(0, 5) as $i) {
21+
$process = proc_open('echo $$ > /dev/null', [], $pipes);
22+
$pid = proc_get_status($process)['pid'];
23+
$processes[$pid] = $process;
24+
}
25+
26+
$iters = 50;
27+
while (!empty($processes) && $iters > 0) {
28+
usleep(100_000);
29+
$iters--;
30+
}
31+
32+
var_dump(empty($processes));
33+
?>
34+
--EXPECT--
35+
bool(true)

0 commit comments

Comments
 (0)