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 296fad1 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.

Author: nicolas-grekas

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)

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 */

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--
+<?php
+
+pcntl_async_signals(true);
+
+pcntl_signal(SIGUSR1, function ($signo) {
+    echo "Handler called\n";
+});
+
+try {
+    zend_test_raise_and_throw(SIGUSR1);
+} catch (\Exception $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+Handler called
+Exception after raise()

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--
+<?php
+
+pcntl_signal(SIGUSR1, function ($signo) {
+    echo "Handler called from ", debug_backtrace()[1]['function'], "()\n";
+});
+
+pcntl_async_signals(true);
+
+function test() {
+    declare(ticks=1) {
+        register_tick_function('zend_test_raise_and_throw', SIGUSR1);
+    }
+    unregister_tick_function('zend_test_raise_and_throw');
+}
+
+test();
+
+?>
+--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

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 <signal.h>
 
 // `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

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 */