diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dcc37f..b149c8c 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 83f42d7..2d86eb9 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 3f4bd8d..95a0a5a 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 2fa44a2..dc4df12 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 a2827e2..b5ba2e8 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 0000000..75f3c3c --- /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