diff --git a/NEWS b/NEWS index 113e79508dbd..8d34aad3fe67 100644 --- a/NEWS +++ b/NEWS @@ -65,6 +65,12 @@ 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) + . 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 column index. (Ilia Alshanetsky) diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c index 082bdc4ba90e..b55e1417d114 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,8 +1349,24 @@ 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) { + 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 +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); } } @@ -1375,17 +1393,44 @@ 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(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; + 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 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)