Skip to content

Commit

Permalink
Revert the fix for GH-11498
Browse files Browse the repository at this point in the history
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.

Closes GH-11863.
  • Loading branch information
nielsdos committed Aug 3, 2023
1 parent 162bd2a commit f7be15d
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 53 deletions.
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ PHP NEWS
. Avoid adding an unnecessary read-lock when loading script from shm if
restart is in progress. (mikhainin)

- PCNTL:
. Revert behaviour of receiving SIGCHLD signals back to the behaviour
before 8.1.22. (nielsdos)

- Standard:
. Prevent int overflow on $decimals in number_format. (Marc Bennewitz)

Expand Down
58 changes: 8 additions & 50 deletions ext/pcntl/pcntl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--TEST--
GH-11498 (SIGCHLD is not always returned from proc_open)
Waiting on SIGCHLD with a pcntl_wait() loop
--EXTENSIONS--
pcntl
--SKIPIF--
Expand All @@ -14,8 +14,10 @@ $processes = [];

pcntl_async_signals(true);
pcntl_signal(SIGCHLD, function($sig, $info) use (&$processes) {
echo "SIGCHLD\n";
unset($processes[$info['pid']]);
while (($pid = pcntl_wait($status, WUNTRACED | WNOHANG)) > 0) {
echo "SIGCHLD\n";
unset($processes[$pid]);
}
}, false);

for ($i = 0; $i <= 5; $i++) {
Expand Down

0 comments on commit f7be15d

Please sign in to comment.