From 42dea58a4b862d397b063fe1c6fd73f1830b40d3 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Thu, 17 Sep 2026 14:30:16 +0300 Subject: [PATCH] Hand graceful_shutdown()'s cancellation to the coroutines (#302) The PHP function parsed $cancellationError and then called ZEND_ASYNC_SHUTDOWN(), which takes nothing, so cancel_queued_coroutines() always built its own AsyncCancellation("Graceful shutdown"). start_graceful_shutdown_with(cancellation) carries the object into the scheduler; NULL keeps the default, and the API entry point start_graceful_shutdown() wraps it, so the ABI is unchanged. The second cancellation pass in finally_shutdown() stays on the default. Test: tests/edge_cases/015-graceful_shutdown_custom_cancellation.phpt, red without the change. ext/async/tests 1220 passed, fuzzy-tests/_generated 724 passed. --- CHANGELOG.md | 2 + async.c | 2 +- async.stub.php | 4 ++ scheduler.c | 20 +++++++-- scheduler.h | 10 +++++ ...graceful_shutdown_custom_cancellation.phpt | 41 +++++++++++++++++++ 6 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 tests/edge_cases/015-graceful_shutdown_custom_cancellation.phpt diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dcc37ff..b149c8c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **`graceful_shutdown()` ignored its `$cancellationError` argument** (#302). The argument was parsed and dropped: `cancel_queued_coroutines()` built its own `AsyncCancellation("Graceful shutdown")` and every coroutine received that one, so a caller passing `new ServerShutdown('...')` could not tell its own shutdown from the default. The PHP function now hands the object to the scheduler, which cancels the coroutines with it and falls back to the default only when none was given; a shutdown already in progress keeps its own cancellation. Evidence: `tests/edge_cases/015-graceful_shutdown_custom_cancellation.phpt`, red before and green after. + - **A throwing `CURLOPT_FNMATCH_FUNCTION` or `CURLOPT_SSH_HOSTKEYFUNCTION` callback escaped the coroutine** (#299). The exception stayed in `EG(exception)` instead of being carried on the async event, so the awaiter was woken with a cancellation and the throw surfaced as an uncaught fatal error. Both callbacks now use the handoff the other curl callbacks already have. Evidence: `tests/curl/072-fnmatch_exception.phpt`; the host-key half has no test, the development machine's libcurl carries neither `sftp` nor `scp`. Needs php-src `4c9e0be9480`. - **A curl upload's read state pointed at memory it did not own** (#295, #294). The subscription on the stream's IO event counted nothing — `zend_async_callbacks_push` counts the callback, and the stream raises its own `ref_count` only around a parked coroutine — so an `fclose()` between two of curl's reads freed the io and `curl_async_read_state_free()` then unsubscribed through the freed block. The subscription holds a reference of its own now, keyed on the callback rather than on the pointer: a build against libcurl below 8.11.1 has `file.io` without `file.io_cb`. Separately, a `CURLFile` part's read state hangs on the mime callback argument and `curl_async_event_stop()` knew only the handle's, so it went on naming an event about to be freed; the handle owns those arguments, and stop() walks them and severs the back-link, while freeing stays with `curl_async_free_cb()`. Evidence for the first half: `tests/curl/071-upload_stream_closed_mid_read.phpt`, heap-use-after-free under ASAN before and clean after. The second half has no test — the dangling pointer was seen with a read in flight by instrumenting the event's destructor, but no script made a completion land between that destructor and the free_cb, in ten attempts. Also removed: `CURL_READ_OWNS_FD`, a flag set nowhere since `CURLFile` moved to `php_stream`, whose two `close()` branches would have closed a descriptor the stream owns. Needs php-src `d7177dc3f65`. diff --git a/async.c b/async.c index 83f42d73..2d86eb90 100644 --- a/async.c +++ b/async.c @@ -956,7 +956,7 @@ PHP_FUNCTION(Async_graceful_shutdown) THROW_IF_ASYNC_OFF; THROW_IF_SCHEDULER_CONTEXT; - ZEND_ASYNC_SHUTDOWN(); + start_graceful_shutdown_with(cancellation); } PHP_FUNCTION(Async_iterate) diff --git a/async.stub.php b/async.stub.php index 3f4bd8d9..95a0a5aa 100644 --- a/async.stub.php +++ b/async.stub.php @@ -264,6 +264,10 @@ function spawn_thread(\Closure $task, bool $inherit = true, ?\Closure $bootloade /** * Start the graceful shutdown of the Scheduler. + * + * Every running coroutine is cancelled with $cancellationError; null selects + * the default "Graceful shutdown" cancellation. A shutdown already in progress + * keeps its own cancellation and the argument is ignored. */ function graceful_shutdown(?AsyncCancellation $cancellationError = null): void {} diff --git a/scheduler.c b/scheduler.c index 2fa44a2e..dc4df120 100644 --- a/scheduler.c +++ b/scheduler.c @@ -887,7 +887,8 @@ static bool resolve_deadlocks(void) /////////////////////////////////////////////////////////// /// SHUTDOWN AND CLEANUP /////////////////////////////////////////////////////////// -static void cancel_queued_coroutines(void) +/* cancellation: the exception handed to every coroutine, or NULL for the default one. */ +static void cancel_queued_coroutines(zend_object *cancellation) { zend_object **exception = &EG(exception); zend_object *prev_exception_storage = NULL; @@ -898,7 +899,13 @@ static void cancel_queued_coroutines(void) // 1. Walk through all coroutines and cancel them if they are suspended. zval *current; - zend_object *cancellation_exception = async_new_exception(async_ce_cancellation_exception, "Graceful shutdown"); + zend_object *cancellation_exception = cancellation; + + if (cancellation_exception != NULL) { + GC_ADDREF(cancellation_exception); + } else { + cancellation_exception = async_new_exception(async_ce_cancellation_exception, "Graceful shutdown"); + } ZEND_ASYNC_SCHEDULER_CONTEXT = true; @@ -991,6 +998,11 @@ void bailout_all_coroutines(void) } bool start_graceful_shutdown(void) +{ + return start_graceful_shutdown_with(NULL); +} + +bool start_graceful_shutdown_with(zend_object *cancellation) { if (ZEND_ASYNC_GRACEFUL_SHUTDOWN) { return true; @@ -1009,7 +1021,7 @@ bool start_graceful_shutdown(void) zend_clear_exception(); } - cancel_queued_coroutines(); + cancel_queued_coroutines(cancellation); if (UNEXPECTED(EG(exception) != NULL)) { zend_exception_set_previous(EG(exception), ZEND_ASYNC_EXIT_EXCEPTION); @@ -1031,7 +1043,7 @@ static void finally_shutdown(void) zend_clear_exception(); } - cancel_queued_coroutines(); + cancel_queued_coroutines(NULL); execute_queued_coroutines(); ZEND_ASYNC_SCHEDULER_CONTEXT = true; diff --git a/scheduler.h b/scheduler.h index a2827e23..b5ba2e84 100644 --- a/scheduler.h +++ b/scheduler.h @@ -28,6 +28,16 @@ void async_scheduler_shutdown(void); bool start_graceful_shutdown(void); +/** + * Start the graceful shutdown with a caller-supplied cancellation. + * + * Every running coroutine is cancelled with `cancellation`, an instance of + * Async\AsyncCancellation owned by the caller; NULL selects the default + * "Graceful shutdown" exception. A shutdown already in progress keeps its + * own cancellation and the argument is ignored. + */ +bool start_graceful_shutdown_with(zend_object *cancellation); + bool async_scheduler_launch(void); /** * A function that is called when control needs to be transferred from a coroutine to the Scheduler. diff --git a/tests/edge_cases/015-graceful_shutdown_custom_cancellation.phpt b/tests/edge_cases/015-graceful_shutdown_custom_cancellation.phpt new file mode 100644 index 00000000..75f3c3c3 --- /dev/null +++ b/tests/edge_cases/015-graceful_shutdown_custom_cancellation.phpt @@ -0,0 +1,41 @@ +--TEST-- +Async\graceful_shutdown(): the cancellation passed in reaches the coroutines +--FILE-- +getMessage(), "\n"; + } +}); + +spawn(function () { + suspend(); + graceful_shutdown(new ServerShutdown('Server shutdown')); +}); + +try { + suspend(); + suspend(); +} catch (AsyncCancellation $e) { + echo "main: ", get_class($e), "\n"; +} + +echo "end\n"; + +?> +--EXPECT-- +main: ServerShutdown +end +ServerShutdown: Server shutdown