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