From 0b4037eba93c4eb24871fd537de3f73c6f91bf95 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 25 Sep 2026 14:16:30 +0200 Subject: [PATCH 1/2] ext/pcntl: run signal handlers when dispatch happens with an exception pending ZEND_DO_FCALL runs its interrupt check right after an internal function returns, before the pending exception is handled, and zend_call_function() does the same for the calls it makes, so pcntl_interrupt_function() can reach 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 whole queue without a single handler having run. The signal is destroyed rather than delayed: a later pcntl_signal_dispatch() finds nothing left. Set the exception aside while the handlers run and chain it back afterwards. The frame is left as found, EG(opline_before_exception) included: depending on the caller, the exception may not be registered on it yet, or may already be on its way to a catch block, and the caller finishes the job once we return. zend_test_raise_and_throw() and the user frame test are from Arnaud Le Blanc. Any long blocking internal call that throws on timeout reaches this. pecl/amqp throws "Consumer timeout exceed" out of AMQPQueue::consume(), which makes a Symfony messenger worker miss every SIGTERM whatever the timeout is. PDO/SQLite throws "database is locked" once busy_timeout expires, which kills a keepalive SIGALRM for the rest of the process's life. --- NEWS | 4 +++ ext/pcntl/pcntl.c | 30 +++++++++++++++++ ...ntl_signal_dispatch_exception_pending.phpt | 24 ++++++++++++++ ...dispatch_exception_pending_user_frame.phpt | 33 +++++++++++++++++++ ext/zend_test/test.c | 17 ++++++++++ ext/zend_test/test.stub.php | 2 ++ ext/zend_test/test_arginfo.h | 8 ++++- 7 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 ext/pcntl/tests/pcntl_signal_dispatch_exception_pending.phpt create mode 100644 ext/pcntl/tests/pcntl_signal_dispatch_exception_pending_user_frame.phpt diff --git a/NEWS b/NEWS index 113e79508dbd..311358e16c91 100644 --- a/NEWS +++ b/NEWS @@ -65,6 +65,10 @@ PHP NEWS is_cacheable_stream_path()). (ndossche) . Fix zend_analyze_calls() call_stack buffer overrun. (Mrmaxmeier) +- PCNTL: + . Fixed pcntl_signal_dispatch() dropping the queued signals when it runs while + an exception is pending. (nicolas-grekas) + - PDO: . Fixed PDOStatement::getColumnMeta() reading out of bounds for an invalid column index. (Ilia Alshanetsky) diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c index 082bdc4ba90e..09d044cd947e 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,9 @@ void pcntl_signal_dispatch(void) { zval params[2], *handle, retval; struct php_pcntl_pending_signal *queue, *next; + zend_object *old_exception; + const zend_op *old_opline_before_exception = NULL; + const zend_op *old_opline = NULL; sigset_t mask; sigset_t old_mask; @@ -1345,6 +1349,20 @@ void pcntl_signal_dispatch(void) PCNTL_G(head) = NULL; /* simple stores are atomic */ PCNTL_G(tail) = NULL; + /* Dispatching can happen with an exception pending, e.g. from the interrupt check that runs + * right after an internal function threw. call_user_function() does nothing in that state, + * so set the exception aside while the handlers run. The frame is left as found: depending + * on the caller, the exception may not be registered on it yet, or may already be on its + * way to a catch block, and the caller takes it from there once we return. */ + old_exception = EG(exception); + if (old_exception) { + if (EG(current_execute_data)) { + old_opline = EG(current_execute_data)->opline; + } + old_opline_before_exception = EG(opline_before_exception); + EG(exception) = NULL; + } + /* Allocate */ while (queue) { if ((handle = zend_hash_index_find(&PCNTL_G(php_signal_table), queue->signo)) != NULL) { @@ -1385,6 +1403,18 @@ void pcntl_signal_dispatch(void) queue = next; } + if (old_exception) { + if (EG(current_execute_data)) { + EG(current_execute_data)->opline = old_opline; + } + EG(opline_before_exception) = old_opline_before_exception; + if (EG(exception)) { + zend_exception_set_previous(EG(exception), old_exception); + } else { + EG(exception) = old_exception; + } + } + PCNTL_G(pending_signals) = 0; /* Re-enable queue */ diff --git a/ext/pcntl/tests/pcntl_signal_dispatch_exception_pending.phpt b/ext/pcntl/tests/pcntl_signal_dispatch_exception_pending.phpt new file mode 100644 index 000000000000..5a7968e9b382 --- /dev/null +++ b/ext/pcntl/tests/pcntl_signal_dispatch_exception_pending.phpt @@ -0,0 +1,24 @@ +--TEST-- +pcntl_signal_dispatch() runs the handlers of the signals raised while an internal function ran and then threw +--EXTENSIONS-- +pcntl +zend_test +--FILE-- +getMessage(), "\n"; +} + +?> +--EXPECT-- +Handler called +Exception after raise() diff --git a/ext/pcntl/tests/pcntl_signal_dispatch_exception_pending_user_frame.phpt b/ext/pcntl/tests/pcntl_signal_dispatch_exception_pending_user_frame.phpt new file mode 100644 index 000000000000..bae00348c144 --- /dev/null +++ b/ext/pcntl/tests/pcntl_signal_dispatch_exception_pending_user_frame.phpt @@ -0,0 +1,33 @@ +--TEST-- +pcntl_signal_dispatch() with an exception pending after an internal function called from a user frame +--EXTENSIONS-- +pcntl +zend_test +--FILE-- + +--EXPECTF-- +Handler called from test() + +Fatal error: Uncaught Exception: Exception after raise() in %s:%d +Stack trace: +#0 %s(%d): zend_test_raise_and_throw(%d) +#1 %s(%d): test() +#2 {main} + thrown in %s on line %d diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 576bacd5e4a9..c4be7ff42fb6 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -37,6 +37,7 @@ #include "zend_call_stack.h" #include "zend_exceptions.h" #include "zend_mm_custom_handlers.h" +#include // `php.h` sets `NDEBUG` when not `PHP_DEBUG` which will make `assert()` from // assert.h a no-op. In order to have `assert()` working on NDEBUG builds, we @@ -672,6 +673,22 @@ static ZEND_FUNCTION(zend_test_crash) php_printf("%s", invalid); } +static ZEND_FUNCTION(zend_test_raise_and_throw) +{ + zend_long signo; + + ZEND_PARSE_PARAMETERS_START(1, 1) + Z_PARAM_LONG(signo) + ZEND_PARSE_PARAMETERS_END(); + + if (raise((int) signo) != 0) { + zend_throw_error(NULL, "raise() failed"); + RETURN_THROWS(); + } + + zend_throw_exception(NULL, "Exception after raise()", 0); +} + static bool has_opline(zend_execute_data *execute_data) { return execute_data diff --git a/ext/zend_test/test.stub.php b/ext/zend_test/test.stub.php index 9116245c30f4..dfe04caabbb4 100644 --- a/ext/zend_test/test.stub.php +++ b/ext/zend_test/test.stub.php @@ -298,6 +298,8 @@ function zend_get_map_ptr_last(): int {} function zend_test_crash(?string $message = null): void {} + function zend_test_raise_and_throw(int $signal): void {} + function zend_test_fill_packed_array(array &$array): void {} /** @return resource */ diff --git a/ext/zend_test/test_arginfo.h b/ext/zend_test/test_arginfo.h index 039757207e69..c08feb900466 100644 --- a/ext/zend_test/test_arginfo.h +++ b/ext/zend_test/test_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: bf65e1dd1eeeeec46687a76a7ea6554cd1971dfc */ + * Stub hash: d87db0f02d40732749355ee65828221c3fd14cdb */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0) ZEND_END_ARG_INFO() @@ -133,6 +133,10 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_crash, 0, 0, IS_VOID, ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, message, IS_STRING, 1, "null") ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_raise_and_throw, 0, 1, IS_VOID, 0) + ZEND_ARG_TYPE_INFO(0, signal, IS_LONG, 0) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_fill_packed_array, 0, 1, IS_VOID, 0) ZEND_ARG_TYPE_INFO(1, array, IS_ARRAY, 0) ZEND_END_ARG_INFO() @@ -294,6 +298,7 @@ static ZEND_FUNCTION(zend_test_zend_call_stack_use_all); static ZEND_FUNCTION(zend_test_is_string_marked_as_valid_utf8); static ZEND_FUNCTION(zend_get_map_ptr_last); static ZEND_FUNCTION(zend_test_crash); +static ZEND_FUNCTION(zend_test_raise_and_throw); static ZEND_FUNCTION(zend_test_fill_packed_array); static ZEND_FUNCTION(zend_test_create_throwing_resource); static ZEND_FUNCTION(get_open_basedir); @@ -401,6 +406,7 @@ static const zend_function_entry ext_functions[] = { ZEND_FE(zend_test_is_string_marked_as_valid_utf8, arginfo_zend_test_is_string_marked_as_valid_utf8) ZEND_FE(zend_get_map_ptr_last, arginfo_zend_get_map_ptr_last) ZEND_FE(zend_test_crash, arginfo_zend_test_crash) + ZEND_FE(zend_test_raise_and_throw, arginfo_zend_test_raise_and_throw) ZEND_FE(zend_test_fill_packed_array, arginfo_zend_test_fill_packed_array) ZEND_FE(zend_test_create_throwing_resource, arginfo_zend_test_create_throwing_resource) ZEND_FE(get_open_basedir, arginfo_get_open_basedir) From 6ea4da73cab26472c93fa29a9af618248d486d4b Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 25 Sep 2026 14:16:30 +0200 Subject: [PATCH 2/2] ext/pcntl: keep the signals a throwing handler left in the queue When a handler threw, the signals queued behind it were recycled without ever being delivered. Put them back on the queue instead, and re-arm the interrupt so that the engine dispatches them once the exception has been handled, rather than leaving them to wait for another signal to come in. Not calling further handlers while the exception propagates is unchanged. --- NEWS | 2 + ext/pcntl/pcntl.c | 37 ++++++++++----- .../pcntl_signal_dispatch_exception_2.phpt | 44 +++++++++++++++++ .../pcntl_signal_dispatch_exception_3.phpt | 47 +++++++++++++++++++ 4 files changed, 119 insertions(+), 11 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 311358e16c91..8d34aad3fe67 100644 --- a/NEWS +++ b/NEWS @@ -68,6 +68,8 @@ PHP NEWS - PCNTL: . Fixed pcntl_signal_dispatch() dropping the queued signals when it runs while an exception is pending. (nicolas-grekas) + . Fixed pcntl_signal_dispatch() dropping the signals queued behind a handler + that throws. (nicolas-grekas) - PDO: . Fixed PDOStatement::getColumnMeta() reading out of bounds for an invalid diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c index 09d044cd947e..b55e1417d114 100644 --- a/ext/pcntl/pcntl.c +++ b/ext/pcntl/pcntl.c @@ -1365,6 +1365,8 @@ void pcntl_signal_dispatch(void) /* 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); @@ -1383,9 +1385,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); } } @@ -1393,14 +1393,11 @@ void pcntl_signal_dispatch(void) queue->next = PCNTL_G(spares); PCNTL_G(spares) = queue; queue = next; - } - /* drain the remaining in case of exception thrown */ - while (queue) { - next = queue->next; - 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; + } } if (old_exception) { @@ -1415,7 +1412,25 @@ void pcntl_signal_dispatch(void) } } - PCNTL_G(pending_signals) = 0; + if (UNEXPECTED(queue)) { + /* Put back what the throwing handler did not get to, instead of dropping it, and ask + * the engine to come back once the exception has been handled. Signals are still + * blocked here, so PCNTL_G(head) cannot have been repopulated in the meantime. */ + 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