Fix 73783 : pcntl_signal() not handling SIG_IGN correctly#2246
Closed
bp1222 wants to merge 2 commits intophp:PHP-7.1from
Closed
Fix 73783 : pcntl_signal() not handling SIG_IGN correctly#2246bp1222 wants to merge 2 commits intophp:PHP-7.1from
bp1222 wants to merge 2 commits intophp:PHP-7.1from
Conversation
Bug #73783 raises an issue with signal handling when using SIG_IGN. With PHP7.1 ZEND_SIGNALS is defaulted to on, which will for all signals set the handler as zend_signal_handler_defer. This is problematic for syscalls like sleep(), which will only return when the requisite number of seconds have elapsed, or, a non-ignored signal is raised. In this case we want to SIG_IGN SIGCHLD, however, SIG_IGN is only stored in the SIGG(handlers) array, and the actual system level handler is defined. This prevents proper signal ignoring when requeted.
|
Comment on behalf of krakjoe at php.net: labelling |
Member
|
Patch looks fine to me. Is it possible to add a test for this without jumping through too many hoops? |
Contributor
Author
|
Done. Looking closer, it'd seem to me with ZEND_SIGNALS turned on by default now, pcntl signal stuff could probably itself be wrapped up in a ifndef ZEND_SIGNALS, and have wrapper stuff to just use the zend-signal storage. Seems to be no point in maintaining two sets of signal-handling and tracking code. I could potentially look at this later to clean it up |
Member
|
Merged via b09c2f8, thanks! |
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
3 participants
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.
Bug #73783 raises an issue with signal handling when using SIG_IGN.
With PHP7.1 ZEND_SIGNALS is defaulted to on, which will for all
signals set the handler as zend_signal_handler_defer. This is
problematic for syscalls like sleep(), which will only return when the
requisite number of seconds have elapsed, or, a non-ignored signal is
raised. In this case we want to SIG_IGN SIGCHLD, however, SIG_IGN is
only stored in the SIGG(handlers) array, and the actual system level
handler is defined. This prevents proper signal ignoring when requeted.