diff --git a/CHANGELOG.md b/CHANGELOG.md index bc22415..5a87eb9 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 +- **A stream write refused at submit read as a write that went out.** The completion of a fire-and-forget write carries no status, so its `free_cb` reads the verdict from `ZEND_ASYNC_IO_WRITE_FAILED` on the handle; a write that `uv_write` refused at once, one over the 0x7ffff000-byte cap or the platform's buffer limit, a vectored write with more than 65535 slots, with unsupported flags or on a closed or non-stream handle all handed the buffer back with the flag clear. Every refusal now sets the flag before the buffer is handed back. true-async-server counts such a TLS write in `tls_bytes_ciphertext_out_total` until its own completion reads the flag. A write to a closed handle never returned its buffer at all, while `writev` on one did; it does now. Evidence: `sapi/asynctest/tests/test_write_refused.c` in php-src, 7 of its 8 cases fail on `c95dcf7` and all pass here, 3 runs; without the two flags at the `uv_write` errors, the two cases that write to a read-only pipe fail. + - **A stream write's completion wrote into a request its listener had freed, and the next write on that handle crashed** (#310). `io_pipe_write_cb` cleared `req->io` after notifying the handle's listeners. A listener that disposes the request inside the notify and submits the next write there, as true-async-server's log sink does, frees it at once, and the next request takes the same address; the store after the notify then cleared `io` on that in-flight request, and its completion dereferenced a NULL handle. The awaited `writev` completion had the same order. Both now detach the request before the notify. Evidence: `sapi/asynctest/tests/test_write_resubmit.c` (true-async/php-src) crashes 3 of 3 on `ef85610` and passes here; true-async-server `core/030` built with `--enable-http-server-test-hooks` crashed 2 of 2 and passes 5 of 5. - **`stop()` on a listen event did not stop accepting** (#305). libuv has no call that undoes `uv_listen`, and `libuv_listen_stop` only dropped the event's reference count, so a stopped listener went on accepting and every connection reached its callbacks: true-async-server, which pauses its listeners at its connection cap, answered each connection that arrived during a pause with 503 instead of leaving it in the kernel backlog, 5 of 5 runs. A stopped listener now leaves the next connection unaccepted; libuv then stops watching the socket on Unix and posts no new AcceptEx on Windows, so the rest of the queue waits in the backlog. A later `start()` delivers the held connection on the next loop tick, never from inside `start()`, and a callback that pauses again during that delivery stops it. A Windows TCP listener posts one AcceptEx at a time instead of libuv's 32: libuv keeps completed accepts in a LIFO stack, so a pause that held several handed the newest over first, and on the server's Windows CI a keep-alive connection that overtook an older one kept it waiting past its 5 s timeout (`h1/064`). With one, the listener holds at most one connection and the rest wait in the kernel backlog in arrival order, and worker threads that share one socket get an even share; the cost is one accept per loop iteration, not measured. A failed `uv_accept` on Unix no longer leaves the socket unwatched: libuv restarts the watcher only after a successful one, and the listener now restarts it itself. Evidence: `sapi/asynctest/tests/test_listen_pause.c` in php-src, 5 of 5 cases red before and green after (3 of 3 runs); true-async-server's `h1/062`, `h1/063` and `h1/064` answer 503 before and pass 10 of 10 after. diff --git a/libuv_reactor.c b/libuv_reactor.c index 73e75c3..1d694ee 100644 --- a/libuv_reactor.c +++ b/libuv_reactor.c @@ -6257,6 +6257,17 @@ static inline bool libuv_can_use_sync_io(void) * in libuv's private header, so it is repeated rather than included. */ #define ASYNC_IO_WRITE_MAX_BYTES 0x7ffff000u +/* {{{ libuv_io_refuse_write + * Record on the handle that a write was refused at submit. Called before the + * caller's buffer is handed back: a refused fire-and-forget write reaches its + * free_cb from inside the submit call, and the flag is the only verdict that + * callback can read. */ +static zend_always_inline void libuv_io_refuse_write(zend_async_io_t *io_base) +{ + io_base->state |= ZEND_ASYNC_IO_WRITE_FAILED; +} +/* }}} */ + static zend_always_inline bool async_uv_buf_set(uv_buf_t *buf, char *base, const size_t len) { #ifdef PHP_WIN32 @@ -6502,8 +6513,16 @@ static zend_async_io_req_t *libuv_io_write(zend_async_io_t *io_base, const char { async_io_t *io = (async_io_t *) io_base; + /* A fire-and-forget caller gave the buffer away at the call, so it gets it + * back here as on every other refusal; writev does the same. */ if (UNEXPECTED(io->base.state & ZEND_ASYNC_IO_CLOSED)) { + libuv_io_refuse_write(io_base); async_throw_error("Cannot write to closed IO handle"); + + if (free_cb != NULL) { + free_cb((void *) buf, io_base); + } + return NULL; } @@ -6522,6 +6541,7 @@ static zend_async_io_req_t *libuv_io_write(zend_async_io_t *io_base, const char if (UNEXPECTED(free_cb != NULL && count > ASYNC_IO_WRITE_MAX_BYTES)) { async_throw_error("Write of %zu bytes exceeds the %u a single write carries", count, (unsigned) ASYNC_IO_WRITE_MAX_BYTES); + libuv_io_refuse_write(io_base); libuv_io_req_dispose(&req->base); return NULL; } @@ -6550,6 +6570,7 @@ static zend_async_io_req_t *libuv_io_write(zend_async_io_t *io_base, const char if (UNEXPECTED(!async_uv_buf_set(&write_buffer, (char *) buf, req->max_size))) { async_throw_error("Write of %zu bytes exceeds this platform's buffer limit", req->max_size); + libuv_io_refuse_write(io_base); libuv_io_req_dispose(&req->base); return NULL; } @@ -6560,6 +6581,7 @@ static zend_async_io_req_t *libuv_io_write(zend_async_io_t *io_base, const char if (UNEXPECTED(error < 0)) { async_throw_error("Failed to start stream write: %s", uv_strerror(error)); + libuv_io_refuse_write(io_base); libuv_io_req_dispose(&req->base); return NULL; } @@ -6644,6 +6666,8 @@ static zend_async_io_req_t *libuv_io_write(zend_async_io_t *io_base, const char static void libuv_writev_release(zend_async_io_t *io_base, const void *bufs, const unsigned nbufs, const bool iov_mode, zend_async_io_write_free_cb_t free_cb, void *user_data) { + libuv_io_refuse_write(io_base); + if (iov_mode) { if (free_cb != NULL) { free_cb(user_data, io_base); @@ -6766,6 +6790,7 @@ static zend_async_io_req_t *libuv_io_writev(zend_async_io_t *io_base, if (UNEXPECTED(!fits)) { free_alloca(tmp, tmp_heap); async_throw_error("Vectored write buffer exceeds this platform's limit"); + libuv_io_refuse_write(io_base); libuv_io_req_dispose(&req->base); return NULL; } @@ -6779,6 +6804,7 @@ static zend_async_io_req_t *libuv_io_writev(zend_async_io_t *io_base, if (UNEXPECTED(error < 0)) { async_throw_error("Failed to start vectored write: %s", uv_strerror(error)); + libuv_io_refuse_write(io_base); /* dispose handles both modes uniformly (writev_nbufs branch for ZSTR, * free_cb branch for IOV). */ libuv_io_req_dispose(&req->base);