From 25cbd20136f5cb1931a1ee3e223d9f67c35145ec Mon Sep 17 00:00:00 2001 From: Edmond <1571649+EdmondDantes@users.noreply.github.com> Date: Sun, 24 May 2026 17:20:48 +0000 Subject: [PATCH 1/3] #144 libuv: notify parked reader on close (no-wake hang) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit libuv_io_close ran uv_read_stop without notifying the parked active_req, so closing a STREAM-type IO handle (PIPE/TCP/TTY) while another coroutine was parked in fread() (via uv_read_start) silently dropped the read watcher. The reader waited forever on an event that never fired again; the deadlock detector eventually aborted the request. Repro: proc_open + a coroutine parked in fread($pipes[1]); killer calls proc_terminate + proc_close — proc_open's resource dtor closes the pipe through the PHP stream → php_stdiop_on_async_detach → libuv_io_close. Fix: before uv_read_stop, mark the parked req completed (transferred=0), set the EOF state bit, and ZEND_ASYNC_CALLBACKS_NOTIFY so the reader wakes with a clean EOF — same shape as the io_pipe_read_cb UV_EOF path. Regression test tests/exec/025-proc_close_wakes_parked_fread.phpt. Full ext/async/tests io/exec/stream suite green on NTS + ASAN-ZTS. Fixes #144 --- CHANGELOG.md | 1 + libuv_reactor.c | 12 ++++++ .../025-proc_close_wakes_parked_fread.phpt | 39 +++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 tests/exec/025-proc_close_wakes_parked_fread.phpt diff --git a/CHANGELOG.md b/CHANGELOG.md index bb186c9..bab7af1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.7.0] - ### Fixed +- **#144 libuv: close-mid-read leaves parked reader hung forever** — `libuv_io_close` ran `uv_read_stop` without notifying the parked `active_req`, so closing a STREAM-type IO handle (PIPE/TCP/TTY) while another coroutine was parked in `fread()` (via `uv_read_start`) silently dropped the read watcher and the reader waited on an event that would never fire again. The deadlock detector eventually aborted the request. Repro: `proc_open` child + a coroutine parked in `fread($pipes[1])`; killer calls `proc_terminate` + `proc_close` — the resource dtor closes the pipe, the reader hangs. Fixed: before `uv_read_stop`, set `req->completed`, `req->transferred=0`, the EOF state bit, and `ZEND_ASYNC_CALLBACKS_NOTIFY` so the reader wakes with a clean EOF. Regression test `tests/exec/025-proc_close_wakes_parked_fread.phpt`. - **#141 Pool deadlock on broken-release with parked waiters** — when `beforeRelease` returned false (e.g. PDO marked the connection broken after a Toxiproxy `reset_peer`), the pool destroyed the resource and decremented `active_count` but never woke a parked waiter. The slot was conceptually free but nothing could claim it: the next acquire path requires a fresh factory call, and only a `release()` wakes waiters. With every connection in flight failing, the pool wedged until the global `Async\DeadlockError` detector fired. Fixed in `zend_async_pool_release` — after destroying a broken resource we now `pool_wake_waiter(pool)`, letting the cascade drain (each waiter retries the factory and propagates its own exception). Defensive fix in `zend_async_pool_acquire`: factory failures on the slot-reservation path now also wake one waiter and throw `PoolException` when the factory returns false silently, so the caller fails fast instead of falling through to `pool_wait_for_resource` with no one able to wake it. - **#138 Stop event once per waker cycle** — multiple coroutines reading the same PHP stream share one cached poll proxy; cancellation went through both `stop_waker_events` (preemptive bulk stop) and `waker_events_dtor` (per-trigger stop) and double-decremented the proxy's `loop_ref_count`. Last cancel ran the LAST-stop body and removed the proxy from the libuv poll list, leaving sibling readers parked forever. Fixed via an `events_stopped:1` bit on `zend_async_waker_t` that the dtor checks; the bit is set by `stop_waker_events` and reset by `start_waker_events`. Each trigger gained a `waker` back-pointer so the dtor reads the bit in O(1). ABI bumped to v0.18.0. See `fuzzy-tests/FINDINGS.md`. diff --git a/libuv_reactor.c b/libuv_reactor.c index 9288ef3..978a9ea 100644 --- a/libuv_reactor.c +++ b/libuv_reactor.c @@ -5103,6 +5103,18 @@ static bool libuv_io_close(zend_async_io_t *io_base) } if (ZEND_ASYNC_IO_IS_STREAM(io->base.type)) { + /* Wake any parked reader with EOF — uv_read_stop drops the + * read watcher silently and would otherwise leave the + * coroutine waiting on an event that no longer fires. */ + async_io_req_t *parked = io->active_req; + if (parked != NULL) { + io->active_req = NULL; + parked->base.transferred = 0; + parked->base.completed = true; + io->base.state |= ZEND_ASYNC_IO_EOF; + ZEND_ASYNC_CALLBACKS_NOTIFY(&io->base.event, &parked->base, NULL); + } + uv_read_stop(&io->handle.stream); io->handle.stream.data = io; ZEND_ASYNC_EVENT_ADD_REF(&io->base.event); diff --git a/tests/exec/025-proc_close_wakes_parked_fread.phpt b/tests/exec/025-proc_close_wakes_parked_fread.phpt new file mode 100644 index 0000000..28a4d65 --- /dev/null +++ b/tests/exec/025-proc_close_wakes_parked_fread.phpt @@ -0,0 +1,39 @@ +--TEST-- +proc_close wakes parked fread on child stdout pipe (regression: #144) +--SKIPIF-- + +--FILE-- + ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes); + +$r = spawn(function() use ($pipes) { + $d = @fread($pipes[1], 4096); + echo "fread returned ", var_export($d, true), "\n"; +}); + +$k = spawn(function() use ($proc) { + delay(50); + proc_terminate($proc, 15); + proc_close($proc); +}); + +await_all([$r, $k]); +echo "OK\n"; +?> +--EXPECT-- +fread returned '' +OK From 4abf2fc12481c3af556bd40a6bbc8f3f5e937fa2 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+EdmondDantes@users.noreply.github.com> Date: Sun, 24 May 2026 17:47:38 +0000 Subject: [PATCH 2/3] Revert "#144 libuv: notify parked reader on close (no-wake hang)" This reverts commit 25cbd20136f5cb1931a1ee3e223d9f67c35145ec. --- CHANGELOG.md | 1 - libuv_reactor.c | 12 ------ .../025-proc_close_wakes_parked_fread.phpt | 39 ------------------- 3 files changed, 52 deletions(-) delete mode 100644 tests/exec/025-proc_close_wakes_parked_fread.phpt diff --git a/CHANGELOG.md b/CHANGELOG.md index bab7af1..bb186c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.7.0] - ### Fixed -- **#144 libuv: close-mid-read leaves parked reader hung forever** — `libuv_io_close` ran `uv_read_stop` without notifying the parked `active_req`, so closing a STREAM-type IO handle (PIPE/TCP/TTY) while another coroutine was parked in `fread()` (via `uv_read_start`) silently dropped the read watcher and the reader waited on an event that would never fire again. The deadlock detector eventually aborted the request. Repro: `proc_open` child + a coroutine parked in `fread($pipes[1])`; killer calls `proc_terminate` + `proc_close` — the resource dtor closes the pipe, the reader hangs. Fixed: before `uv_read_stop`, set `req->completed`, `req->transferred=0`, the EOF state bit, and `ZEND_ASYNC_CALLBACKS_NOTIFY` so the reader wakes with a clean EOF. Regression test `tests/exec/025-proc_close_wakes_parked_fread.phpt`. - **#141 Pool deadlock on broken-release with parked waiters** — when `beforeRelease` returned false (e.g. PDO marked the connection broken after a Toxiproxy `reset_peer`), the pool destroyed the resource and decremented `active_count` but never woke a parked waiter. The slot was conceptually free but nothing could claim it: the next acquire path requires a fresh factory call, and only a `release()` wakes waiters. With every connection in flight failing, the pool wedged until the global `Async\DeadlockError` detector fired. Fixed in `zend_async_pool_release` — after destroying a broken resource we now `pool_wake_waiter(pool)`, letting the cascade drain (each waiter retries the factory and propagates its own exception). Defensive fix in `zend_async_pool_acquire`: factory failures on the slot-reservation path now also wake one waiter and throw `PoolException` when the factory returns false silently, so the caller fails fast instead of falling through to `pool_wait_for_resource` with no one able to wake it. - **#138 Stop event once per waker cycle** — multiple coroutines reading the same PHP stream share one cached poll proxy; cancellation went through both `stop_waker_events` (preemptive bulk stop) and `waker_events_dtor` (per-trigger stop) and double-decremented the proxy's `loop_ref_count`. Last cancel ran the LAST-stop body and removed the proxy from the libuv poll list, leaving sibling readers parked forever. Fixed via an `events_stopped:1` bit on `zend_async_waker_t` that the dtor checks; the bit is set by `stop_waker_events` and reset by `start_waker_events`. Each trigger gained a `waker` back-pointer so the dtor reads the bit in O(1). ABI bumped to v0.18.0. See `fuzzy-tests/FINDINGS.md`. diff --git a/libuv_reactor.c b/libuv_reactor.c index 978a9ea..9288ef3 100644 --- a/libuv_reactor.c +++ b/libuv_reactor.c @@ -5103,18 +5103,6 @@ static bool libuv_io_close(zend_async_io_t *io_base) } if (ZEND_ASYNC_IO_IS_STREAM(io->base.type)) { - /* Wake any parked reader with EOF — uv_read_stop drops the - * read watcher silently and would otherwise leave the - * coroutine waiting on an event that no longer fires. */ - async_io_req_t *parked = io->active_req; - if (parked != NULL) { - io->active_req = NULL; - parked->base.transferred = 0; - parked->base.completed = true; - io->base.state |= ZEND_ASYNC_IO_EOF; - ZEND_ASYNC_CALLBACKS_NOTIFY(&io->base.event, &parked->base, NULL); - } - uv_read_stop(&io->handle.stream); io->handle.stream.data = io; ZEND_ASYNC_EVENT_ADD_REF(&io->base.event); diff --git a/tests/exec/025-proc_close_wakes_parked_fread.phpt b/tests/exec/025-proc_close_wakes_parked_fread.phpt deleted file mode 100644 index 28a4d65..0000000 --- a/tests/exec/025-proc_close_wakes_parked_fread.phpt +++ /dev/null @@ -1,39 +0,0 @@ ---TEST-- -proc_close wakes parked fread on child stdout pipe (regression: #144) ---SKIPIF-- - ---FILE-- - ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], - $pipes); - -$r = spawn(function() use ($pipes) { - $d = @fread($pipes[1], 4096); - echo "fread returned ", var_export($d, true), "\n"; -}); - -$k = spawn(function() use ($proc) { - delay(50); - proc_terminate($proc, 15); - proc_close($proc); -}); - -await_all([$r, $k]); -echo "OK\n"; -?> ---EXPECT-- -fread returned '' -OK From 4f9745dcbb80bc37cde179f3cae24d7f3f876d7f Mon Sep 17 00:00:00 2001 From: Edmond <1571649+EdmondDantes@users.noreply.github.com> Date: Mon, 25 May 2026 08:09:17 +0000 Subject: [PATCH 3/3] #144 libuv: notify parked subscribers before tearing watcher down MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit libuv_io_close used to call uv_read_stop / uv_close without notifying subscribers on io->base.event. A coroutine parked in fread() / fwrite() on a stream that the owner is about to free would hang forever; the deadlock detector eventually aborted, and an eventual resume UAFed on the freed stream (the resource dtor pefree'd both stream and abstract data while we waited). Stream owns the io event. Before tearing the watcher down, walk the subscribers: build an InputOutputException, mark the in-flight active_req io_closed (so php_stdiop_read / _write early-return without touching the dying stream — see php-src companion PR), and ZEND_ASYNC_CALLBACKS_NOTIFY the event so every parked reader/writer wakes. Covers STREAM types (PIPE/TCP/TTY) and UDP — active_req can hold either async_io_req_t or async_udp_req_t, type-aware cast picks the right base. Regression test tests/exec/025-proc_close_wakes_parked_fread.phpt. Local: ASAN-debug-ZTS pass 30/30 reruns on minimal repro; full ext/async/tests/{exec,io,stream} green. Requires php-src PR (zend_async_io_req_t.io_closed + plain_wrapper early-return). ABI v0.18 → v0.19. Fixes #144 --- CHANGELOG.md | 1 + libuv_reactor.c | 24 ++++++++++ .../025-proc_close_wakes_parked_fread.phpt | 45 +++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 tests/exec/025-proc_close_wakes_parked_fread.phpt diff --git a/CHANGELOG.md b/CHANGELOG.md index bb186c9..000a704 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.7.0] - ### Fixed +- **#144 libuv: close-mid-read leaves parked reader hung forever** — `libuv_io_close` ran `uv_read_stop` without notifying the parked `active_req`. Closing a STREAM-type IO handle (PIPE/TCP/TTY) while a coroutine was parked in `fread()` silently dropped the read watcher; the reader hung until the deadlock detector aborted the request, then UAFed on the freed stream when it finally resumed (the resource dtor had `pefree`'d both stream and abstract data while it waited). Typical trigger: `proc_open` + a coroutine reading the child's stdout pipe + another coroutine calling `proc_close`. Fixed by walking event subscribers before tearing the watcher down: `libuv_io_close` now marks the active req `io_closed`, builds an `InputOutputException`, and `ZEND_ASYNC_CALLBACKS_NOTIFY`s the event so every parked reader/writer wakes. ABI bumped to v0.19.0 — `zend_async_io_req_t` and `zend_async_udp_req_t` gained a `bool io_closed` field so consumers (`php_stdiop_read`/`php_stdiop_write` in php-src) can early-return without touching the freed stream. Regression test `tests/exec/025-proc_close_wakes_parked_fread.phpt`. - **#141 Pool deadlock on broken-release with parked waiters** — when `beforeRelease` returned false (e.g. PDO marked the connection broken after a Toxiproxy `reset_peer`), the pool destroyed the resource and decremented `active_count` but never woke a parked waiter. The slot was conceptually free but nothing could claim it: the next acquire path requires a fresh factory call, and only a `release()` wakes waiters. With every connection in flight failing, the pool wedged until the global `Async\DeadlockError` detector fired. Fixed in `zend_async_pool_release` — after destroying a broken resource we now `pool_wake_waiter(pool)`, letting the cascade drain (each waiter retries the factory and propagates its own exception). Defensive fix in `zend_async_pool_acquire`: factory failures on the slot-reservation path now also wake one waiter and throw `PoolException` when the factory returns false silently, so the caller fails fast instead of falling through to `pool_wait_for_resource` with no one able to wake it. - **#138 Stop event once per waker cycle** — multiple coroutines reading the same PHP stream share one cached poll proxy; cancellation went through both `stop_waker_events` (preemptive bulk stop) and `waker_events_dtor` (per-trigger stop) and double-decremented the proxy's `loop_ref_count`. Last cancel ran the LAST-stop body and removed the proxy from the libuv poll list, leaving sibling readers parked forever. Fixed via an `events_stopped:1` bit on `zend_async_waker_t` that the dtor checks; the bit is set by `stop_waker_events` and reset by `start_waker_events`. Each trigger gained a `waker` back-pointer so the dtor reads the bit in O(1). ABI bumped to v0.18.0. See `fuzzy-tests/FINDINGS.md`. diff --git a/libuv_reactor.c b/libuv_reactor.c index 9288ef3..f416c57 100644 --- a/libuv_reactor.c +++ b/libuv_reactor.c @@ -5102,6 +5102,30 @@ static bool libuv_io_close(zend_async_io_t *io_base) goto close_orig_fd; } + /* Wake parked subscribers — stream owner is about to free its abstract + * state. Mark active_req io_closed so consumers skip stream-side access + * after resume. See #144. */ + if (io->base.event.callbacks.length > 0) { + zend_object *exc = async_new_exception( + async_ce_input_output_exception, "Stream was closed"); + if (io->active_req != NULL) { + if (io->base.type == ZEND_ASYNC_IO_TYPE_UDP) { + async_udp_req_t *ureq = (async_udp_req_t *) io->active_req; + ureq->base.io_closed = true; + ureq->base.completed = true; + ureq->base.transferred = 0; + } else { + io->active_req->base.io_closed = true; + io->active_req->base.completed = true; + io->active_req->base.transferred = 0; + } + io->active_req = NULL; + } + io->base.state |= ZEND_ASYNC_IO_EOF; + ZEND_ASYNC_CALLBACKS_NOTIFY(&io->base.event, NULL, exc); + OBJ_RELEASE(exc); + } + if (ZEND_ASYNC_IO_IS_STREAM(io->base.type)) { uv_read_stop(&io->handle.stream); io->handle.stream.data = io; diff --git a/tests/exec/025-proc_close_wakes_parked_fread.phpt b/tests/exec/025-proc_close_wakes_parked_fread.phpt new file mode 100644 index 0000000..3b30de8 --- /dev/null +++ b/tests/exec/025-proc_close_wakes_parked_fread.phpt @@ -0,0 +1,45 @@ +--TEST-- +proc_close wakes parked fread on child stdout pipe (regression: #144) +--SKIPIF-- + +--FILE-- + ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes); + +$r = spawn(function() use ($pipes) { + $d = @fread($pipes[1], 4096); + echo "fread returned ", var_export($d, true), "\n"; +}); + +$k = spawn(function() use ($proc) { + delay(50); + proc_terminate($proc, 15); + proc_close($proc); +}); + +await_all([$r, $k]); +echo "OK\n"; +?> +--EXPECT-- +fread returned false +OK