ext/pcntl: do not drop queued signals when an exception is pending - #23624
Open
nicolas-grekas wants to merge 1 commit into
Open
ext/pcntl: do not drop queued signals when an exception is pending#23624nicolas-grekas wants to merge 1 commit into
nicolas-grekas wants to merge 1 commit into
Conversation
pcntl_signal_dispatch() takes the whole queue out of PCNTL_G(head) before it starts calling handlers, and recycles every entry it walks over. Two paths let signals disappear that way. The first one is the interrupt handler. ZEND_VM_FCALL_INTERRUPT_CHECK() runs right after an internal function returns, before the pending exception is handled, so pcntl_interrupt_function() reaches the dispatcher with EG(exception) set. call_user_function() returns without calling anything in that state, the "if (EG(exception)) break" added by 296fad1 fires on the first entry, and the drain loop then recycles the entire queue without a single handler having run. Setting the exception aside for the duration of the dispatch, the way zend_lookup_class_ex() does around autoloading, lets the handlers run. The second one is a handler that throws while other signals are queued behind it: those were recycled too. They now go back to the queue, so the next dispatch delivers them. This is reachable from any long blocking internal call that throws on timeout. pecl/amqp is one: AMQPQueue::consume() throws "Consumer timeout exceed" when the read timeout expires, which made a Symfony messenger worker built on it miss every SIGTERM, whatever the timeout was.
nicolas-grekas
force-pushed
the
pcntl-signal-queue-drop
branch
from
September 9, 2026 06:30
23bfbd0 to
36d571c
Compare
Member
|
Thanks I ll have a look some time in the following days, I would say tough after a quick look there are incoming changes needed. |
Member
|
Is #22538 related? |
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.
pcntl_signal_dispatch()detaches the whole queue fromPCNTL_G(head)before it starts calling handlers, and recycles every entry it walks over. Two paths let queued signals disappear.1. Dispatching while an exception is pending
ZEND_VM_FCALL_INTERRUPT_CHECK()runs right after an internal function returns, before the pending exception is handled. So when an internal function throws,pcntl_interrupt_function()reaches the dispatcher withEG(exception)set. There,zend_call_function()returns without calling anything, theif (EG(exception)) break;added by 296fad1 fires on the very first entry, and the/* drain the remaining */loop recycles the entire queue. No handler ever runs, and nothing is left for a laterpcntl_signal_dispatch()either.Any signal that arrives while such a function is running is therefore lost, not delayed. The fix sets the exception aside for the duration of the dispatch, the way
zend_lookup_class_ex()already does around autoloading.2. Signals queued behind a throwing handler
If a handler throws, the signals queued behind it were recycled as well. They now go back to the queue so the next dispatch delivers them. The existing behaviour of not calling further handlers while the exception propagates is unchanged, and
pcntl_signal_dispatch_exception.phptstill passes as is.How I ran into it
Any long blocking internal call that throws on timeout hits case 1. pecl/amqp is one:
AMQPQueue::consume()throwsAMQPQueueException("Consumer timeout exceed")when the connection read timeout expires. A Symfony Messenger worker built on it (symfony/symfony#65920) misses everySIGTERM, whatever the timeout is, because the worker spends essentially all of its wall time inside that call:653 iterations of a 10 ms call and the signal is never delivered, not even by an explicit
pcntl_signal_dispatch()afterwards.SigBlk,SigIgnandSigCgtread from inside the extension's callback confirm the signal is neither blocked nor ignored and that the handler is installed, and the same signal duringsleep(),stream_select(),stream_socket_accept()orsocket_read()is delivered normally. What makesconsume()different is only that it throws: when a message arrives and it returns normally instead, the signal is delivered.With this patch, same build, same test, no userland workaround:
I verified this against pecl/amqp 2.2.0 built on this branch, with and without the patch. Case 2 is covered by the added
.phpt; case 1 needs an internal function that blocks long enough for a signal to arrive and then throws, which I could not build out of core functions alone, since re-entering the VM for any userland callback dispatches the signal first.ext/pcntl,Zend/testsandext/standard/testsare green (8180 passed, 0 failed).