fix(threadpool): per-task nursery scope for sync tasks — snapshot UAF (no scope-pointer swap)

CHANGELOG.md

@@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Fixed
+- **`ThreadPool` synchronous task: snapshot use-after-free when the task spawns an un-awaited coroutine** — a sync-mode task body ran inline in the worker and its per-task snapshot arena (which backs every spawned closure's op_array) was freed the instant the body returned, while a coroutine the body had spawned was still pending; running it later dereferenced freed memory (Windows debug-heap crash; ASAN-caught on Linux). The body now runs as a coroutine in its own per-task **nursery** `Scope`: `Async\spawn()` inside the body lands in that scope on its own (no scope-pointer hijacking), and on task exit the scope is cancelled and *drained* — awaited until every spawned coroutine is physically disposed — before the snapshot is freed. ABI bumped to v0.20.0: new `zend_async_scope_await_after_cancellation_fn` exposes the C core of `Scope::awaitAfterCancellation` so the worker reuses the canonical zombie-aware drain instead of hand-rolling it. Regression test `tests/thread_pool/065-task_scope_nursery_no_uaf.phpt`.
+- **`ThreadPool`: fatal in a task no longer leaves a use-after-free or a leaked libuv loop** — a fatal (e.g. OOM) in a task body now rejects the future with `ThreadTransferException` and tears the pool down cleanly. The snapshot's op_array name strings (`function_name`, `filename`) are materialized into refcounted heap strings so holders that outlive the snapshot arena (the closure, `PG(last_error_file)`) are freed by refcount instead of dangling. The `zend_bailout()` that a fatal re-raises through a parked `ThreadChannel` send/recv or the worker's slot wait is now caught so the channel/slot trigger is disposed before re-raising — an undisposed trigger's open `uv_async` would block `uv_loop_close` and leak the reactor's loop internals. Debug builds dump any libuv handle that survives reactor shutdown. Regression tests `tests/thread_pool/066`–`068`.
+- **`ThreadPool` coroutine-mode task: a fatal now reports its cause instead of resolving to `null`** — in `coroutine: true` mode a task that hit a fatal/OOM (or `exit()`/`die()`) resolved its future to a silent `null`, because the completion path only checked for a thrown exception and a bailout is not one. It now detects the bailout (no exception, `UNDEF` result) and rejects the future with a `ThreadTransferException` carrying the fatal message, matching synchronous mode. Tests `tests/thread_pool/067`, `068`.
+- **`spawn_thread`/`ThreadPool`: cross-thread use-after-free of a closure's captured-variable names** — a transferred closure's `use`-variable names were copied with `zend_string_dup()`, which returns interned strings unchanged; the variable names are interned in the parent thread, so the worker's snapshot held pointers into the parent's interned-string table. When the parent aborted/ended its request (`zend_interned_strings_deactivate`) while a worker was still building the closure, the worker read freed memory in `async_thread_create_closure` (ASAN `heap-use-after-free`). The keys are now copied into private persistent strings owned by the snapshot. Regression tests `tests/thread/050`, `052`.
 - **`TaskSet`/`TaskGroup(scope: $scope)` use-after-free on teardown** — the group held only an event refcount on a PHP-supplied scope, which is shared with coroutine bookkeeping, so a finishing coroutine could free the scope while `group->scope` still pointed at it (`task_group.c:486`, seen as `zend_mm_heap corrupted`). The group now holds a strong ref to the external Scope object for its lifetime, so the scope can't be disposed while in use. Test `tests/task_group/043-task_group_external_scope_uaf.phpt`.
 
 ## [0.7.0] - 2026-06-02

async_API.c

@@ -1304,6 +1304,7 @@ void async_api_register(void)
 								  async_scheduler_coroutine_enqueue,
 								  async_coroutine_resume,
 								  async_coroutine_cancel,
+								  async_scope_await_after_cancellation,
 								  async_spawn_and_throw,
 								  start_graceful_shutdown,
 								  async_waker_new,

libuv_reactor.c

@@ -411,6 +411,18 @@ static void libuv_reactor_stop_with_exception(void)
 
 /* }}} */
 
+#ifdef ZEND_DEBUG
+/* Dump a libuv handle that survived reactor shutdown — such a handle keeps the
+ * loop from closing (uv_loop_close => EBUSY) and leaks the loop's internals. */
+static void libuv_debug_dump_handle(uv_handle_t *handle, void *arg)
+{
+	(void) arg;
+	fprintf(stderr, "async: leftover libuv handle: type=%s active=%d closing=%d has_ref=%d\n",
+		uv_handle_type_name(uv_handle_get_type(handle)),
+		uv_is_active(handle), uv_is_closing(handle), uv_has_ref(handle));
+}
+#endif
+
 /* {{{ libuv_reactor_shutdown */
 bool libuv_reactor_shutdown(void)
 {
@@ -434,9 +446,20 @@ bool libuv_reactor_shutdown(void)
 		if (uv_loop_alive(UVLOOP)) {
 #ifdef ZEND_DEBUG
 			fprintf(stderr, "async: libuv shutdown timeout; loop left open\n");
+			uv_walk(UVLOOP, libuv_debug_dump_handle, NULL);
 #endif
 		} else {
-			uv_loop_close(UVLOOP);
+			/* uv_loop_close fails with EBUSY if any handle is still open (even an
+			 * unref'd one, which uv_loop_alive ignores) — that would silently
+			 * leak the loop's internals. Surface it in debug builds. */
+			const int close_result = uv_loop_close(UVLOOP);
+			if (UNEXPECTED(close_result != 0)) {
+#ifdef ZEND_DEBUG
+				fprintf(stderr, "async: uv_loop_close failed (%s); leftover handles:\n",
+					uv_err_name(close_result));
+				uv_walk(UVLOOP, libuv_debug_dump_handle, NULL);
+#endif
+			}
 		}
 
 		ASYNC_G(reactor_started) = false;

scope.c

@@ -371,95 +371,115 @@ METHOD(awaitCompletion)
 	zend_async_waker_clean(current_coroutine);
 }
 
-METHOD(awaitAfterCancellation)
+/* C core of Scope::awaitAfterCancellation, also used by the thread pool via the
+ * async API. Suspends `awaiter` until the scope is COMPLETELY_DONE (active and
+ * zombie counts both zero — physical disposal, not just "completed"). `awaiter`
+ * must not belong to `scope`. `error_fci`/`cancellation` are optional. */
+void async_scope_await_after_cancellation(
+		zend_async_scope_t *zend_scope, zend_coroutine_t *awaiter,
+		zend_fcall_info *error_fci, zend_fcall_info_cache *error_fci_cache,
+		zend_async_event_t *cancellation)
 {
-	zend_fcall_info error_handler_fci = { 0 };
-	zend_fcall_info_cache error_handler_fcc = { 0 };
-	zend_object *cancellation_obj = NULL;
-
-	ZEND_PARSE_PARAMETERS_START(0, 2)
-	Z_PARAM_OPTIONAL
-	Z_PARAM_FUNC_OR_NULL(error_handler_fci, error_handler_fcc)
-	Z_PARAM_OBJ_OR_NULL(cancellation_obj)
-	ZEND_PARSE_PARAMETERS_END();
-
-	// Mark cancellation token as used immediately, before any early returns
-	if (cancellation_obj != NULL) {
-		zend_async_event_t *cancellation_event = ZEND_ASYNC_OBJECT_TO_EVENT(cancellation_obj);
-		ZEND_ASYNC_EVENT_SET_RESULT_USED(cancellation_event);
-		ZEND_ASYNC_EVENT_SET_EXC_CAUGHT(cancellation_event);
-	}
-
-	zend_coroutine_t *current_coroutine = ZEND_ASYNC_CURRENT_COROUTINE;
-	if (UNEXPECTED(current_coroutine == NULL)) {
+	if (UNEXPECTED(awaiter == NULL || zend_scope == NULL || ZEND_ASYNC_SCOPE_IS_CLOSED(zend_scope))) {
 		return;
 	}
 
-	async_scope_object_t *scope_object = THIS_SCOPE;
-	if (UNEXPECTED(scope_object->scope == NULL || ZEND_ASYNC_SCOPE_IS_CLOSED(&scope_object->scope->scope))) {
-		return;
-	}
-
-	if (false == ZEND_ASYNC_SCOPE_IS_CANCELLED(&scope_object->scope->scope)) {
-		async_throw_error("Attempt to await a Scope that has not been cancelled");
-	}
+	async_scope_t *scope = (async_scope_t *) zend_scope;
 
-	// Check for deadlock: current coroutine belongs to this scope or its children
-	if (async_scope_contains_coroutine(scope_object->scope, current_coroutine, 0)) {
+	// Deadlock guard: the awaiter must not belong to this scope or its children.
+	if (async_scope_contains_coroutine(scope, awaiter, 0)) {
 		async_throw_error(
 				"Cannot await completion of scope from a coroutine that belongs to the same scope or its children");
-		RETURN_THROWS();
+		return;
 	}
 	if (UNEXPECTED(EG(exception))) {
-		RETURN_THROWS();
+		return;
 	}
 
-	// Check if scope is already finished (no active coroutines and no child scopes)
-	if (scope_object->scope->coroutines.length == 0 && scope_object->scope->scope.scopes.length == 0) {
+	// Already drained — no active coroutines and no child scopes.
+	if (scope->coroutines.length == 0 && scope->scope.scopes.length == 0) {
 		return;
 	}
 
-	ZEND_ASYNC_WAKER_NEW(current_coroutine);
+	ZEND_ASYNC_WAKER_NEW(awaiter);
 	if (UNEXPECTED(EG(exception))) {
-		RETURN_THROWS();
+		return;
 	}
 
-	// We need to create a custom callback to handle errors coming from coroutines.
+	// Resumes only when COMPLETELY_DONE; routes any child error through the handler.
 	scope_coroutine_callback_t *scope_callback = (scope_coroutine_callback_t *) zend_async_coroutine_callback_new(
-			current_coroutine, callback_resolve_when_zombie_completed, sizeof(scope_coroutine_callback_t));
+			awaiter, callback_resolve_when_zombie_completed, sizeof(scope_coroutine_callback_t));
 	if (UNEXPECTED(scope_callback == NULL)) {
-		ZEND_ASYNC_WAKER_DESTROY(current_coroutine);
-		RETURN_THROWS();
+		ZEND_ASYNC_WAKER_DESTROY(awaiter);
+		return;
 	}
 
-	if (error_handler_fci.size != 0) {
-		scope_callback->error_fci = &error_handler_fci;
-		scope_callback->error_fci_cache = &error_handler_fcc;
+	if (error_fci != NULL && error_fci->size != 0) {
+		scope_callback->error_fci = error_fci;
+		scope_callback->error_fci_cache = error_fci_cache;
 	} else {
 		scope_callback->error_fci = NULL;
 		scope_callback->error_fci_cache = NULL;
 	}
 
-	if (UNEXPECTED(!zend_async_resume_when(current_coroutine, &scope_object->scope->scope.event, false, NULL,
-		&scope_callback->callback))) {
-		ZEND_ASYNC_WAKER_DESTROY(current_coroutine);
-		RETURN_THROWS();
+	if (UNEXPECTED(!zend_async_resume_when(
+			awaiter, &zend_scope->event, false, NULL, &scope_callback->callback))) {
+		ZEND_ASYNC_WAKER_DESTROY(awaiter);
+		return;
 	}
 
-	if (cancellation_obj != NULL) {
-		zend_async_resume_when(current_coroutine,
-							   ZEND_ASYNC_OBJECT_TO_EVENT(cancellation_obj),
-							   false,
-							   zend_async_waker_callback_cancel,
-							   NULL);
+	if (cancellation != NULL) {
+		zend_async_resume_when(awaiter, cancellation, false, zend_async_waker_callback_cancel, NULL);
 		if (UNEXPECTED(EG(exception))) {
-			zend_async_waker_clean(current_coroutine);
-			RETURN_THROWS();
+			zend_async_waker_clean(awaiter);
+			return;
 		}
 	}
 
 	ZEND_ASYNC_SUSPEND();
-	zend_async_waker_clean(current_coroutine);
+	zend_async_waker_clean(awaiter);
+}
+
+METHOD(awaitAfterCancellation)
+{
+	zend_fcall_info error_handler_fci = { 0 };
+	zend_fcall_info_cache error_handler_fcc = { 0 };
+	zend_object *cancellation_obj = NULL;
+
+	ZEND_PARSE_PARAMETERS_START(0, 2)
+	Z_PARAM_OPTIONAL
+	Z_PARAM_FUNC_OR_NULL(error_handler_fci, error_handler_fcc)
+	Z_PARAM_OBJ_OR_NULL(cancellation_obj)
+	ZEND_PARSE_PARAMETERS_END();
+
+	// Mark cancellation token as used immediately, before any early returns
+	zend_async_event_t *cancellation_event = NULL;
+	if (cancellation_obj != NULL) {
+		cancellation_event = ZEND_ASYNC_OBJECT_TO_EVENT(cancellation_obj);
+		ZEND_ASYNC_EVENT_SET_RESULT_USED(cancellation_event);
+		ZEND_ASYNC_EVENT_SET_EXC_CAUGHT(cancellation_event);
+	}
+
+	zend_coroutine_t *current_coroutine = ZEND_ASYNC_CURRENT_COROUTINE;
+	if (UNEXPECTED(current_coroutine == NULL)) {
+		return;
+	}
+
+	async_scope_object_t *scope_object = THIS_SCOPE;
+	if (UNEXPECTED(scope_object->scope == NULL || ZEND_ASYNC_SCOPE_IS_CLOSED(&scope_object->scope->scope))) {
+		return;
+	}
+
+	if (false == ZEND_ASYNC_SCOPE_IS_CANCELLED(&scope_object->scope->scope)) {
+		async_throw_error("Attempt to await a Scope that has not been cancelled");
+		RETURN_THROWS();
+	}
+
+	async_scope_await_after_cancellation(
+			&scope_object->scope->scope, current_coroutine,
+			error_handler_fci.size != 0 ? &error_handler_fci : NULL,
+			error_handler_fci.size != 0 ? &error_handler_fcc : NULL,
+			cancellation_event);
 }
 
 METHOD(isFinished)

scope.h

@@ -83,6 +83,12 @@ void async_register_scope_ce(void);
 /* Check if coroutine belongs to this scope or any of its child scopes */
 bool async_scope_contains_coroutine(async_scope_t *scope, zend_coroutine_t *coroutine, uint32_t depth);
 
+/* C core of Scope::awaitAfterCancellation (see scope.c). */
+void async_scope_await_after_cancellation(
+		zend_async_scope_t *scope, zend_coroutine_t *awaiter,
+		zend_fcall_info *error_fci, zend_fcall_info_cache *error_fci_cache,
+		zend_async_event_t *cancellation);
+
 void async_scope_notify_coroutine_finished(async_coroutine_t *coroutine);
 
 /* Mark coroutine as zombie and update active count */

tests/thread_pool/065-task_scope_nursery_no_uaf.phpt

@@ -0,0 +1,46 @@
+--TEST--
+ThreadPool: a coroutine spawned but never awaited inside a sync task cannot outlive the per-task snapshot (UAF regression)
+--SKIPIF--
+<?php
+if (!PHP_ZTS) die('skip ZTS required');
+if (!class_exists('Async\ThreadPool')) die('skip ThreadPool not available');
+?>
+--FILE--
+<?php
+/*
+ * Regression: a ThreadPool task deep-copies its closure (and every nested
+ * closure) into a per-task snapshot arena. The worker used to free that arena
+ * the moment the task body returned — while a coroutine the task had spawned
+ * was still pending. The pending coroutine's op_array lived in the just-freed
+ * arena, so running it dereferenced freed memory (Windows debug-heap crash;
+ * ASAN-caught on Linux).
+ *
+ * The fix runs each task body as a coroutine in its own per-task Scope (a
+ * nursery): on task exit the scope is cancelled and drained — awaited until
+ * every spawned coroutine is physically disposed — before the snapshot is
+ * freed. The only guaranteed invariant is that no spawned coroutine outlives
+ * the snapshot; whether such a coroutine got to run at all is timing-dependent
+ * and deliberately NOT asserted. The test passes iff there is no use-after-free
+ * (caught by ASAN / the debug heap), the future resolves, and nothing hangs.
+ */
+use Async\ThreadPool;
+use function Async\spawn;
+use function Async\await;
+use function Async\delay;
+
+$pool = new ThreadPool(1);
+
+$f = $pool->submit(function () {
+    // Spawned, never awaited: still pending when the task body returns, so the
+    // worker must cancel and drain it before freeing this task's snapshot.
+    spawn(function () { delay(10000); });
+    return 'task-done';
+});
+
+var_dump(await($f));
+
+$pool->close();
+echo "ok\n";
+--EXPECT--
+string(9) "task-done"
+ok

tests/thread_pool/066-task_fatal_rejects_future.phpt

@@ -0,0 +1,45 @@
+--TEST--
+ThreadPool: a fatal (OOM) in a sync task rejects its future with ThreadTransferException (no hang/UAF/leak)
+--SKIPIF--
+<?php
+if (!PHP_ZTS) die('skip ZTS required');
+if (!class_exists('Async\ThreadPool')) die('skip ThreadPool not available');
+?>
+--INI--
+memory_limit=64M
+--FILE--
+<?php
+/*
+ * Regression: a fatal in a sync task body re-raises zend_bailout() out of the
+ * task coroutine and longjmps to the worker's bailout handler, past the normal
+ * future-resolution. The handler must still reject the in-flight task's future
+ * (already dequeued, so draining the channel doesn't reach it) — otherwise the
+ * awaiter hangs.
+ *
+ * It must also free the per-task snapshot without a use-after-free: the loaded
+ * op_array's name strings (function_name, filename) are materialized into normal
+ * refcounted heap strings, so holders that outlive the snapshot arena — the
+ * closure freed at request shutdown, and PG(last_error_file) — keep them alive
+ * via refcount instead of dereferencing the freed arena.
+ */
+use Async\ThreadPool;
+use function Async\await;
+
+$pool = new ThreadPool(1);
+
+$f = $pool->submit(function () {
+    $s = str_repeat('x', 500 * 1024 * 1024); // exceeds memory_limit -> bailout
+    return strlen($s);
+});
+
+try {
+    var_dump(await($f));
+} catch (\Throwable $e) {
+    printf("%s: %s\n", get_class($e),
+        str_contains($e->getMessage(), 'memory size') ? 'memory exhausted' : 'other');
+}
+
+echo "done\n";
+--EXPECTF--
+%AAsync\ThreadTransferException: memory exhausted
+done

tests/thread_pool/067-coroutine_task_fatal_no_trigger_leak.phpt

@@ -0,0 +1,43 @@
+--TEST--
+ThreadPool: a fatal in a coroutine-mode task delivers the cause and disposes the worker's channel trigger (no libuv loop leak)
+--SKIPIF--
+<?php
+if (!PHP_ZTS) die('skip ZTS required');
+if (!class_exists('Async\ThreadPool')) die('skip ThreadPool not available');
+?>
+--INI--
+memory_limit=64M
+--FILE--
+<?php
+/*
+ * Regression: in coroutine mode the worker parks on the task-channel receive
+ * (creating a uv_async trigger) while task coroutines run. A fatal in a task
+ * re-raises zend_bailout() through that SUSPEND, which used to skip the
+ * trigger's dispose — leaving an open uv_async that blocked uv_loop_close, so
+ * the libuv loop leaked. The channel now disposes the trigger on bailout
+ * (no-leak verified by LeakSanitizer).
+ *
+ * Also asserts the cause is delivered: pool_task_dispose detects the bailout
+ * (no exception, UNDEF result) and rejects the future with the fatal message
+ * instead of resolving to a silent null.
+ */
+use Async\ThreadPool;
+use function Async\await;
+
+$pool = new ThreadPool(1, 0, null, true); // coroutine mode
+
+$f = $pool->submit(function () {
+    $s = str_repeat('x', 500 * 1024 * 1024); // exceeds memory_limit -> bailout
+    return strlen($s);
+});
+
+try {
+    var_dump(await($f));
+} catch (\Throwable $e) {
+    printf("%s: %s\n", get_class($e),
+        str_contains($e->getMessage(), 'memory size') ? 'memory exhausted' : 'other');
+}
+echo "done\n";
+--EXPECTF--
+%AAsync\ThreadTransferException: memory exhausted
+done