Repository navigation
Fix zombie process on process destruction - #86
Merged
Merged
Conversation
With sufficient luck, another callback installs something at a file description reusing this resources file descriptor number, wreaking havoc. The only reference to $extraDataPipe is stored by extraDataPipeCallbackId. An explicit fclose is the worst choice. Either immediately cancel $callbackId, or simply defer to __destruct. Deferring to destructor seems safe enough.
xtrime-ru
force-pushed
the
fix-posix-zombie
branch
5 times, most recently
from
August 26, 2026 22:21
47566d0 to
ffd4ab3
Compare
xtrime-ru
changed the title
xtrime-ru
force-pushed
the
fix-posix-zombie
branch
from
August 28, 2026 15:17
ffd4ab3 to
5c32f9f
Compare
xtrime-ru
changed the title
Contributor
Author
|
We tested this patch in production for last 2 weeks without issues. Now it is ready for review. |
kelunik
reviewed
Oct 4, 2026
# Conflicts: # src/Internal/Posix/PosixHandle.php
xtrime-ru
force-pushed
the
fix-posix-zombie
branch
from
October 27, 2026 22:27
5c32f9f to
bb3d541
Compare
Contributor
Author
|
I've rebased this branch over #85, to fix conflict |
# Conflicts: # src/Internal/Posix/PosixHandle.php
Member
|
Thanks. Hopefully this finally fixes this issue once and for all. |
trowski
pushed a commit
that referenced
this pull request
Nov 7, 2026
When a job it waits for is killed by a signal, the wrapper shell reports it on its stderr
("Killed: 9" from bash, which is /bin/sh on macOS), and its stderr is the stderr pipe of
the process. If the process filled that pipe with output nobody read, the shell blocked
writing the notice and never exited, and the blocking waitpid() of reapShell() in kill()
(and in PosixHandle::__destruct() once the exit code was read, since #86) waited for it
forever, holding the event loop that would have drained the pipe.
Discard the stderr of the wait builtin: the process's own stderr is redirected by the
command itself, the notice is the only thing the shell writes there.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch is rebased over #85, to fix conflict. So this PR must be merged after #85
Summary
Ensures that the POSIX wrapper shell created by
proc_open()is reaped after process termination, both with and withoutext-pcntl.Shutdown remains non-blocking while the requested command is still running.
Root cause
proc_get_status($proc)['pid'], stored as$shellPid, identifies the wrapper shell created byproc_open(). This shell is a direct child of PHP.The requested command PID, stored as
$pid, belongs to a process started by the wrapper shell. It is not a direct child of PHP, so waiting for$pidcannot reap the wrapper shell.Shell reaping was deferred through the event loop. If the event loop did not run again after the shell exited, the shell remained a zombie.
When
ext-pcntlwas unavailable, the cleanup path returned without performing any wait operation. By default pcntl is absent in php-fpm.A blocking reap after
kill()also requires checking signal delivery first.ESRCHmeans that the requested process has already exited and the wrapper shell can still be reaped. Other delivery errors must be reported instead of proceeding to a potentially blocking wait.Changes
posix_kill()failures, treatingESRCHas an already-exited process and throwing for other errors.$shellPidafterkill()and after completed process destruction.pcntl_waitpid()withEINTRhandling when PCNTL is available.proc_get_status()to reap the wrapper shell when PCNTL is unavailable.This corrects the POSIX shutdown handling introduced in 6c711ee.
Behavior changes
kill()called while status is notEndedEndedproc_get_status()Our zombie issue resulted from two combined conditions: PHP-FPM lacked PCNTL, so explicit shell reaping was skipped, and
kill()sent SIGKILL without reaping the wrapper shell.Testing