From a85e929e5a6bf21ff4245bdd900767eb84eb2c02 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:00:47 +0000 Subject: [PATCH] fix(thread-pool): report a bootloader failure through the worker's error stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bootloader that threw was kept for a later submit() and delivered to the awaiters of the tasks already queued, and nowhere else. A pool driven as a set of long-lived workers reads neither channel: the HTTP server submits one internal task per worker and awaits it only for completion, so the exception was lost and the pool died with no diagnostic at all. The worker now hands the exception to zend_exception_error() before it closes the pool, which reports it the way any uncaught exception is reported — display_errors, log_errors and error_log all apply. That call adds E_DONT_BAIL, so the worker still reaches the close/reject path. The bailout branch keeps its silence: a fatal error has already printed itself on the way there, and exit() is not an error. --- CHANGELOG.md | 6 ++++ .../thread_pool/063-bootloader_exception.phpt | 6 +++- .../080-bootloader_exception_reported.phpt | 35 +++++++++++++++++++ thread_pool.c | 31 ++++++++++++++-- 4 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 tests/thread_pool/080-bootloader_exception_reported.phpt diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ab07a9d..290eaaa2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to the Async extension for PHP will be documented in this fi The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- **A `ThreadPool` bootloader that threw was never reported.** The worker kept the message for a later `submit()` and rejected the tasks already queued; that was its only trace. A pool driven as a set of long-lived workers reads neither channel — an HTTP server submits one task per worker and awaits it only for completion — so the exception was lost, and a bootloader that failed on its first `require` looked like a pool that started and did nothing. The worker now reports the exception through its own error stream, so `display_errors`, `log_errors` and `error_log` apply, and then closes the pool and rejects the queued tasks as before. + ## [0.9.1] - 2026-08-11 ### Fixed diff --git a/tests/thread_pool/063-bootloader_exception.phpt b/tests/thread_pool/063-bootloader_exception.phpt index d9be9494..cd9c05d2 100644 --- a/tests/thread_pool/063-bootloader_exception.phpt +++ b/tests/thread_pool/063-bootloader_exception.phpt @@ -28,5 +28,9 @@ spawn(function() { $pool->close(); }); ?> ---EXPECT-- +--EXPECTF-- +Fatal error: Uncaught RuntimeException: boot failed! in %s:%d +Stack trace: +%A + thrown in %s on line %d boot failed! diff --git a/tests/thread_pool/080-bootloader_exception_reported.phpt b/tests/thread_pool/080-bootloader_exception_reported.phpt new file mode 100644 index 00000000..3c272aa4 --- /dev/null +++ b/tests/thread_pool/080-bootloader_exception_reported.phpt @@ -0,0 +1,35 @@ +--TEST-- +ThreadPool: a bootloader that throws is reported through the worker's error stream with nothing submitted +--SKIPIF-- + +--FILE-- +isClosed(); $i++) { + \Async\delay(10); + } + + $pool->close(); +}); +?> +--EXPECTF-- +Fatal error: Uncaught RuntimeException: boot failed! in %s:%d +Stack trace: +%A + thrown in %s on line %d diff --git a/thread_pool.c b/thread_pool.c index 03e47c9a..368c59e7 100644 --- a/thread_pool.c +++ b/thread_pool.c @@ -106,6 +106,27 @@ static zend_object *thread_pool_wrap_transfer_error(zend_object *src) (msg != NULL && Z_TYPE_P(msg) == IS_STRING) ? Z_STRVAL_P(msg) : "thread transfer failed"); } +/* Report the bootloader's uncaught exception through this worker's own error + * stream — display_errors and error_log, the same route an uncaught exception + * takes in any other request — and consume EG(exception). + * + * The rejection of pending tasks is the pool's only other channel for a failed + * bootloader, and it reaches nobody when the pool serves as a set of long-lived + * workers (an HTTP server submits one task per worker and never awaits it) or + * when the failure happens before anything is submitted. Without this the whole + * pool dies with no diagnostic at all. */ +static void thread_pool_report_boot_failure(void) +{ + if (EG(exception) == NULL) { + return; + } + + /* E_ERROR is reported, not raised: zend_exception_error adds E_DONT_BAIL, + * so the worker keeps running and reaches the reject/close path below. */ + zend_exception_error(EG(exception), E_ERROR); + zend_clear_exception(); +} + /* task_channel is swapped by reload() and read cross-thread on bailout paths — * always go through the atomic load. */ #define POOL_TASK_CHANNEL(pool) \ @@ -242,7 +263,7 @@ static void thread_pool_worker_handler(zend_async_thread_event_t *event, void *c * as a clean transfer exception (the raw Error's backtrace reaches * into worker-local load state and crashes the awaiter if copied). */ zend_object *boot_ex = thread_pool_wrap_transfer_error(EG(exception)); - zend_clear_exception(); + thread_pool_report_boot_failure(); zval_ptr_dtor(&boot_callable); thread_pool_record_bootloader_error(pool, boot_ex); thread_pool_close(pool); @@ -269,7 +290,9 @@ static void thread_pool_worker_handler(zend_async_thread_event_t *event, void *c /* Bootloader called exit()/die() (unwind-exit token) or hit a fatal * error (bailout). Convert into a transfer exception for every * pending task instead of leaking the token through reject or - * re-raising zend_bailout(), either of which crashes the worker fiber. */ + * re-raising zend_bailout(), either of which crashes the worker fiber. + * Nothing is reported here: a fatal error printed itself on the way + * to the bailout, and exit() is not an error. */ if (EG(exception) != NULL) { zend_clear_exception(); } @@ -286,7 +309,9 @@ static void thread_pool_worker_handler(zend_async_thread_event_t *event, void *c * pending task's awaiter instead of a generic cancellation. */ zend_object *boot_ex = EG(exception); GC_ADDREF(boot_ex); - zend_clear_exception(); + /* Takes over the reference EG(exception) held; ours keeps boot_ex + * alive for the reject below. */ + thread_pool_report_boot_failure(); thread_pool_record_bootloader_error(pool, boot_ex); thread_pool_close(pool); thread_pool_drain_tasks(pool, true, boot_ex);