Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
2 changes: 1 addition & 1 deletion async.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions async.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}

Expand Down
20 changes: 16 additions & 4 deletions scheduler.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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;
Expand Down
10 changes: 10 additions & 0 deletions scheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
41 changes: 41 additions & 0 deletions tests/edge_cases/015-graceful_shutdown_custom_cancellation.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
--TEST--
Async\graceful_shutdown(): the cancellation passed in reaches the coroutines
--FILE--
<?php

use function Async\spawn;
use function Async\suspend;
use function Async\graceful_shutdown;
use Async\AsyncCancellation;

class ServerShutdown extends AsyncCancellation {}

spawn(function () {
try {
while (true) {
suspend();
}
} catch (AsyncCancellation $e) {
echo get_class($e), ": ", $e->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
Loading