Fix proc_close() signal exit code handling#21293
Fix proc_close() signal exit code handling#21293alies-dev wants to merge 3 commits intophp:masterfrom
Conversation
proc_close() previously returned the raw waitpid status when a child process was killed by a signal, making it impossible to distinguish a signal death from a normal exit with the same code. Now follows the bash convention: return 128 + signal_number for signal-terminated processes (e.g. 139 for SIGSEGV, 143 for SIGTERM). Closes phpGH-21292
PHP catches SIGTERM internally and does exit(255), so the child process never actually dies from the signal. Use /bin/sh with kill -9 (SIGKILL) which cannot be caught.
| ?> | ||
| --EXPECT-- | ||
| int(137) | ||
| int(42) No newline at end of file |
There was a problem hiding this comment.
Fixed, thanks!
This made me curious -- the .editorconfig already has insert_final_newline = true, but it's not enforced in CI or on pre-commit hook level. A simple check could catch this automatically. Two options that fit the repo's style:
-
Shell one-liner in CI (zero deps):
git ls-files -z '*.phpt' '*.php' '*.c' '*.h' | xargs -0 -L1 sh -c 'test -z "$(tail -c1 "$0")" || echo "missing final newline: $0"' -
editorconfig-checker— single Go binary, validates all.editorconfigrules at once. Used by several large projects (e.g., Kotlin).
Given how minimal the current CI tooling is, option 1 would probably be the least friction.
Please let me know what do you think -- I can create a PR
There was a problem hiding this comment.
I don't remember all the details, but at one point we tried to run Clang Format but it produces a large diff and apparently also altered the behaviour of code. But the first one is probably the most sensible as a base rule?
Although please provide the alternative when you open a PR as others might have a different opinion. :)
|
Seems like BC break to me... |
|
does it need RFC? |
4 participants
When a child process spawned via
proc_open()is killed by a signal,proc_close()returns a raw/mangled waitpid status instead of a meaningful exit code. This makes it impossible to distinguish between "exited normally with code 11" and "killed by SIGSEGV".The fix adds proper
WIFSIGNALED()/WTERMSIG()handling inproc_open_rsrc_dtor()(whichproc_close()calls internally), returning128 + signal_number— the same convention used by bash and other shells.proc_get_status()already handled this correctly via itssignaled/termsigfields; only theproc_close()return value was broken.Fixes GH-21292
The bug originally was found by using Psalm vimeo/psalm#11679 that uses
composer/xdebug-handler: composer/xdebug-handler#161