From 5bfec105b4801559a1472f7b2650036ca2d73830 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 9 Sep 2026 07:59:48 +0200 Subject: [PATCH] ext/pcntl: do not drop queued signals when an exception is pending 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 296fad10fb4 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 destructors are called during unwinding, lets the handlers run. zend_exception_save() is not usable for that: it goes through the single EG(prev_exception) slot, so a dispatch happening inside an autoloader called with an exception set aside would hand that exception back too early. 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, and with asynchronous signals the interrupt is re-armed, so that the engine delivers them on its own once the exception is handled instead of waiting for another signal to come in. 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. --- NEWS | 4 ++ ext/pcntl/pcntl.c | 52 +++++++++++++++---- .../pcntl_signal_dispatch_exception_2.phpt | 44 ++++++++++++++++ .../pcntl_signal_dispatch_exception_3.phpt | 47 +++++++++++++++++ 4 files changed, 137 insertions(+), 10 deletions(-) create mode 100644 ext/pcntl/tests/pcntl_signal_dispatch_exception_2.phpt create mode 100644 ext/pcntl/tests/pcntl_signal_dispatch_exception_3.phpt diff --git a/NEWS b/NEWS index 3346d38ea898..53cfe3650d72 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,10 @@ PHP NEWS registrations are freed while still reachable from the cycle collector. (Ilia Alshanetsky) +- PCNTL: + . Fixed pcntl_signal_dispatch() dropping queued signals when it runs while an + exception is pending. (nicolas-grekas) + 24 Sep 2026, PHP 8.4.26 diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c index 082bdc4ba90e..e1295906eed4 100644 --- a/ext/pcntl/pcntl.c +++ b/ext/pcntl/pcntl.c @@ -31,6 +31,7 @@ #include "ext/standard/info.h" #include "php_signal.h" #include "php_ticks.h" +#include "zend_exceptions.h" #include "zend_fibers.h" #if defined(HAVE_GETPRIORITY) || defined(HAVE_SETPRIORITY) || defined(HAVE_WAIT3) @@ -1318,6 +1319,7 @@ void pcntl_signal_dispatch(void) { zval params[2], *handle, retval; struct php_pcntl_pending_signal *queue, *next; + zend_object *old_exception; sigset_t mask; sigset_t old_mask; @@ -1345,8 +1347,17 @@ void pcntl_signal_dispatch(void) PCNTL_G(head) = NULL; /* simple stores are atomic */ PCNTL_G(tail) = NULL; + /* Dispatching can happen while an exception is propagating, typically from the interrupt + * check that runs right after an internal function returned with an exception pending. + * Handlers cannot be called in that state, so set the exception aside while they run, + * the way destructors are called during unwinding. */ + old_exception = EG(exception); + EG(exception) = NULL; + /* Allocate */ while (queue) { + bool handler_threw = false; + if ((handle = zend_hash_index_find(&PCNTL_G(php_signal_table), queue->signo)) != NULL) { if (Z_TYPE_P(handle) != IS_LONG) { ZVAL_NULL(&retval); @@ -1365,9 +1376,7 @@ void pcntl_signal_dispatch(void) #ifdef HAVE_STRUCT_SIGINFO_T zval_ptr_dtor(¶ms[1]); #endif - if (EG(exception)) { - break; - } + handler_threw = NULL != EG(exception); } } @@ -1375,17 +1384,40 @@ void pcntl_signal_dispatch(void) queue->next = PCNTL_G(spares); PCNTL_G(spares) = queue; queue = next; + + /* No other handler can be called while the exception propagates */ + if (handler_threw) { + break; + } } - /* drain the remaining in case of exception thrown */ - while (queue) { - next = queue->next; - queue->next = PCNTL_G(spares); - PCNTL_G(spares) = queue; - queue = next; + if (old_exception) { + if (EG(exception)) { + zend_exception_set_previous(EG(exception), old_exception); + } else { + EG(exception) = old_exception; + } } - PCNTL_G(pending_signals) = 0; + if (UNEXPECTED(queue)) { + /* The signals a throwing handler left behind go back to the queue instead of being + * dropped, and the engine is asked to dispatch again once that exception is handled. + * Signals are still blocked here, so PCNTL_G(head) cannot have been repopulated. */ + next = queue; + + while (next->next) { + next = next->next; + } + + PCNTL_G(head) = queue; + PCNTL_G(tail) = next; + + if (PCNTL_G(async_signals)) { + zend_atomic_bool_store_ex(&EG(vm_interrupt), true); + } + } else { + PCNTL_G(pending_signals) = 0; + } /* Re-enable queue */ PCNTL_G(processing_signal_queue) = 0; diff --git a/ext/pcntl/tests/pcntl_signal_dispatch_exception_2.phpt b/ext/pcntl/tests/pcntl_signal_dispatch_exception_2.phpt new file mode 100644 index 000000000000..ebd868df5d4b --- /dev/null +++ b/ext/pcntl/tests/pcntl_signal_dispatch_exception_2.phpt @@ -0,0 +1,44 @@ +--TEST-- +pcntl_signal_dispatch() keeps the signals left in the queue by a throwing handler +--EXTENSIONS-- +pcntl +posix +--FILE-- +getMessage() . "\n"; +} + +echo "Handlers called: " . implode(', ', $called) . "\n"; + +pcntl_signal_dispatch(); + +echo "Handlers called: " . implode(', ', $called) . "\n"; + +?> +--EXPECT-- +Exception in signal handler +Handlers called: SIGUSR1 +Handlers called: SIGUSR1, SIGUSR2, SIGHUP diff --git a/ext/pcntl/tests/pcntl_signal_dispatch_exception_3.phpt b/ext/pcntl/tests/pcntl_signal_dispatch_exception_3.phpt new file mode 100644 index 000000000000..ff877e3c0400 --- /dev/null +++ b/ext/pcntl/tests/pcntl_signal_dispatch_exception_3.phpt @@ -0,0 +1,47 @@ +--TEST-- +pcntl_signal_dispatch() delivers the signals a throwing handler left behind once its exception is handled +--EXTENSIONS-- +pcntl +posix +--FILE-- +getMessage() . "\n"; +} + +// No explicit dispatch: the engine delivers what the throwing handler left behind +// on its own, as soon as the exception has been handled +usleep(1000); + +echo "Handlers called: " . implode(', ', $called) . "\n"; + +?> +--EXPECT-- +Exception in signal handler +Handlers called: SIGUSR1, SIGUSR2, SIGHUP