From ecd4fb77c81624493c903227fa59b7df2e8ffbe4 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:46:35 +0000 Subject: [PATCH 1/6] fix(http1): the reactor owns the frame, so a cancelled write cannot outlive it (#179) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A streamed chunk was written from a buffer the caller owned, and libuv does not copy: uv_write keeps the pointer until its completion callback. The caller freed it when its own wait ended, and once the handler coroutine is cancelled that moment arrives first — the wait is over, the write is still queued. Reproduced: a handler parked writing to a peer with SO_RCVBUF at 4 KiB that never reads, setShutdownTimeout(0) so stop() skips the grace window and cancels at once. The await returns with the request incomplete, dispose defers because the write is in flight, and the caller frees a 20008-byte frame; stamping the block showed the allocator hands the same address straight back. On plaintext the frame now goes out as slots through a vectored write the reactor owns — no copy of the body either, and no threshold. TLS keeps the coalesced copy: tls_push copies into the BIO ring anyway, and a vectored write aimed at the socket would put plaintext on a TLS connection. The whole path is behind ZEND_ASYNC_IO_WRITEV_AWAIT, so the extension still builds and works against a reactor without it. Needs true-async/php-src#24 and true-async/php-async#262. --- src/core/http_connection.c | 80 ++++++++++++++++++ src/core/http_connection.h | 11 +++ src/http1/http1_stream.c | 73 +++++++++++++++- .../030-h1-cancel-while-parked-in-write.phpt | 84 +++++++++++++++++++ 4 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 tests/phpt/server/h1/030-h1-cancel-while-parked-in-write.phpt diff --git a/src/core/http_connection.c b/src/core/http_connection.c index d46602cc..8343c901 100644 --- a/src/core/http_connection.c +++ b/src/core/http_connection.c @@ -1413,6 +1413,86 @@ bool http_connection_send_raw(http_connection_t *conn, } /* }}} */ +#ifdef ZEND_ASYNC_IO_WRITEV_AWAIT +/* {{{ http_connection_send_strv_awaited + * + * Vectored plaintext send the caller waits for. Takes one reference per slot + * and never gives it back: the reactor holds them until libuv is done, which + * is what makes this safe where the awaited single-buffer write is not. There + * the buffer stays the caller's, so the caller frees it when its own wait + * ends — and a cancelled caller's wait ends while the write is still queued. + * + * Returns false on a dead peer or a refused submit, with every reference + * consumed either way. Slots reach the wire in array order. + */ +bool http_connection_send_strv_awaited(http_connection_t *conn, + zend_string *const *bufs, + const unsigned nbufs) +{ + /* nbufs == 0 is the one submit refusal that neither throws nor consumes, + * so it cannot be told apart afterwards. Refuse it before that. */ + ZEND_ASSERT(nbufs > 0); + + if (UNEXPECTED(conn->write_timed_out)) { + for (unsigned i = 0; i < nbufs; i++) { + zend_string_release(bufs[i]); + } + + return false; + } + + const uint32_t write_timeout_ms = conn->write_timeout_ms; + + if (write_timeout_ms > 0 && !http_write_timer_arm(conn, write_timeout_ms)) { + for (unsigned i = 0; i < nbufs; i++) { + zend_string_release(bufs[i]); + } + + return false; + } + + size_t total = 0; + + for (unsigned i = 0; i < nbufs; i++) { + total += ZSTR_LEN(bufs[i]); + } + + bool ok_total = false; + zend_async_io_req_t *req = ZEND_ASYNC_IO_WRITEV_AWAITED(conn->io, bufs, nbufs); + + if (req != NULL) { + const bool ok = async_io_req_await(req, conn->io, write_timeout_ms, + HTTP_IO_REQ_WRITE, conn->log_state); + const bool had_exc = (req->exception != NULL); + + if (had_exc) { + OBJ_RELEASE(req->exception); + req->exception = NULL; + } + + const ssize_t transferred = req->transferred; + req->dispose(req); + ok_total = ok && !had_exc && transferred == (ssize_t)total; + } else { + /* Every refusal past the nbufs guard released the slots already; + * absorb the reactor's exception so one dropped connection does not + * reach the top level. */ + http_absorb_io_submission_exception(conn, __func__); + } + + if (write_timeout_ms > 0) { + http_write_timer_stop(conn); + } + + if (UNEXPECTED(conn->write_timed_out)) { + return false; + } + + return ok_total; +} +/* }}} */ +#endif /* ZEND_ASYNC_IO_WRITEV_AWAIT */ + /* {{{ http_connection_send_str_owned * * Fire-and-forget plaintext send: transfer ownership of @p body to the diff --git a/src/core/http_connection.h b/src/core/http_connection.h index ac52643e..700d5fb4 100644 --- a/src/core/http_connection.h +++ b/src/core/http_connection.h @@ -461,6 +461,17 @@ bool http_connection_send_batched_writev(http_connection_t *conn, bool http_connection_send_strv_owned(http_connection_t *conn, zend_string * const *bufs, unsigned nbufs); +#ifdef ZEND_ASYNC_IO_WRITEV_AWAIT +/* Vectored variant the caller waits for. Each slot is an OWNED zend_string + * reference, consumed in every outcome — success, failure, refused submit, and + * a cancellation while parked. That last one is why it exists: the reactor + * holds the references until libuv is done, so nothing the caller built can be + * freed while a queued write still points at it. + * Plaintext only — same TLS caveat as send_str_owned. Requires nbufs > 0. */ +bool http_connection_send_strv_awaited(http_connection_t *conn, + zend_string * const *bufs, unsigned nbufs); +#endif + /* Outbound backpressure (transport-level, plaintext batched path). * pending_bytes = coalesced tail waiting behind the single in-flight * batched write — the part that grows under a slow consumer. The diff --git a/src/http1/http1_stream.c b/src/http1/http1_stream.c index 84d78bbc..405bd93d 100644 --- a/src/http1/http1_stream.c +++ b/src/http1/http1_stream.c @@ -105,6 +105,35 @@ static void h1_stream_headers_committed(http1_request_ctx_t *ctx) http_server_on_streaming_response_started(ctx->conn->counters); } +#ifdef ZEND_ASYNC_IO_WRITEV_AWAIT +static void h1_stream_headers_committed_if(http1_request_ctx_t *ctx, const bool sent) +{ + if (sent) { + h1_stream_headers_committed(ctx); + } +} +#endif + +#ifdef ZEND_ASYNC_IO_WRITEV_AWAIT +/* The CRLF every chunk frame ends with. Interned, because the vectored send + * takes a reference it will release: an interned string ignores both, so one + * literal serves every frame of every connection. A persistent-but-not-interned + * string would be decremented per frame and freed under the next one. */ +static zend_string *h1_crlf_interned(void) +{ + static zend_string *crlf = NULL; + + if (UNEXPECTED(crlf == NULL)) { + crlf = zend_string_init_interned("\r\n", 2, 1); + ZEND_ASSERT(ZSTR_IS_INTERNED(crlf)); + } + + return crlf; +} + +static void h1_stream_headers_committed_if(http1_request_ctx_t *ctx, bool sent); +#endif + static bool h1_emit_headers_once(http1_request_ctx_t *ctx) { zend_string *headers = h1_streaming_headers_build(ctx); @@ -221,7 +250,49 @@ static int h1_stream_append_chunk(void *opaque, zend_string *chunk, * over — today's ABI offers one or the other, never both. */ const size_t head_len = headers != NULL ? ZSTR_LEN(headers) : 0; const size_t frame_len = head_len + (size_t)header_len + chunk_len + 2; - const bool coalesce = frame_len <= H1_CHUNK_COALESCE_MAX; + +#ifdef ZEND_ASYNC_IO_WRITEV_AWAIT + /* Plaintext: hand the pieces over as slots. One submit, one suspension, no + * copy of the body — and every piece belongs to the reactor until libuv is + * done with it, so a handler cancelled while parked cannot leave a queued + * write pointing at memory its C frame has released. + * + * TLS keeps the coalesced copy below: http_connection_send routes through + * tls_push, which copies into the BIO ring anyway, and a vectored write + * aimed straight at the socket would put plaintext on a TLS connection. */ +#ifdef HAVE_OPENSSL + const bool plaintext = conn->tls == NULL; +#else + const bool plaintext = true; +#endif + + if (plaintext) { + zend_string *slots[4]; + unsigned n = 0; + + if (headers != NULL) { + slots[n++] = headers; /* ownership moves to the reactor */ + headers = NULL; + } + + slots[n++] = zend_string_init(header, (size_t)header_len, 0); + slots[n++] = chunk; /* the caller's ref, handed over */ + slots[n++] = h1_crlf_interned(); + + const bool sent = http_connection_send_strv_awaited(conn, slots, n); + + if (UNEXPECTED(!sent || EG(exception) != NULL)) { + ctx->stream_dead = true; + return HTTP_STREAM_APPEND_STREAM_DEAD; + } + + h1_stream_headers_committed_if(ctx, head_len != 0); + http_server_on_stream_send(conn->counters, chunk_len); + return HTTP_STREAM_APPEND_OK; + } +#endif + + const bool coalesce = frame_len <= H1_CHUNK_COALESCE_MAX; bool frame_ok = true; /* Too large to carry the block along: the headers go out on their own, and diff --git a/tests/phpt/server/h1/030-h1-cancel-while-parked-in-write.phpt b/tests/phpt/server/h1/030-h1-cancel-while-parked-in-write.phpt new file mode 100644 index 00000000..47218a95 --- /dev/null +++ b/tests/phpt/server/h1/030-h1-cancel-while-parked-in-write.phpt @@ -0,0 +1,84 @@ +--TEST-- +HttpResponse::write() — a handler cancelled while parked inside a write unwinds cleanly +--EXTENSIONS-- +true_async_server +true_async +sockets +--FILE-- +addListener('127.0.0.1', $port) + ->setReadTimeout(30)->setWriteTimeout(30) + ->setShutdownTimeout(0)); /* no grace window: cancel at once */ + +$seen = []; + +$server->addHttpHandler(function ($req, $res) use (&$seen) { + $res->setStatusCode(200)->setHeader('Content-Type', 'application/octet-stream'); + + try { + /* Far more than any socket buffer holds; parks after the first chunks. */ + for ($i = 0; $i < 5000; $i++) { + $res->write(str_repeat('x', 20000)); + } + $res->end(); + $seen['outcome'] = 'finished'; + } catch (\Throwable $e) { + $seen['outcome'] = get_class($e) . ':' . $e->getCode(); + $seen['writable_after'] = $res->isWritable(); + } +}); + +$cli = spawn(function () use ($port, $server) { + usleep(50000); + + $fp = stream_socket_client("tcp://127.0.0.1:$port", $errno, $errstr, 5); + $sock = socket_import_stream($fp); + socket_set_option($sock, SOL_SOCKET, SO_RCVBUF, 4096); + + fwrite($fp, "GET /stream HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); + + /* Read nothing at all: the handler parks inside the write. */ + delay(500); + $server->stop(); + delay(500); + fclose($fp); +}); + +$server->start(); +await($cli); + +foreach ($seen as $k => $v) echo "$k = ", var_export($v, true), "\n"; +echo "done\n"; +?> +--EXPECT-- +outcome = 'TrueAsync\\HttpException:499' +writable_after = false +done From 28fb7033e369b41cf64f330b14cbadbe88a02e67 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:00:39 +0000 Subject: [PATCH 2/6] http1: tighten the comments and drop a wrapper around one if (#179) --- src/core/http_connection.c | 20 ++++++-------------- src/core/http_connection.h | 8 +++----- src/http1/http1_stream.c | 33 +++++++++++---------------------- 3 files changed, 20 insertions(+), 41 deletions(-) diff --git a/src/core/http_connection.c b/src/core/http_connection.c index 8343c901..293a4113 100644 --- a/src/core/http_connection.c +++ b/src/core/http_connection.c @@ -1416,21 +1416,15 @@ bool http_connection_send_raw(http_connection_t *conn, #ifdef ZEND_ASYNC_IO_WRITEV_AWAIT /* {{{ http_connection_send_strv_awaited * - * Vectored plaintext send the caller waits for. Takes one reference per slot - * and never gives it back: the reactor holds them until libuv is done, which - * is what makes this safe where the awaited single-buffer write is not. There - * the buffer stays the caller's, so the caller frees it when its own wait - * ends — and a cancelled caller's wait ends while the write is still queued. - * - * Returns false on a dead peer or a refused submit, with every reference - * consumed either way. Slots reach the wire in array order. - */ + * Vectored plaintext send the caller waits for. Slots go out in array order; + * every reference is consumed whatever happens, including a cancellation while + * parked — the reactor holds them until libuv is done, which is exactly what + * the awaited single-buffer write cannot promise. */ bool http_connection_send_strv_awaited(http_connection_t *conn, zend_string *const *bufs, const unsigned nbufs) { - /* nbufs == 0 is the one submit refusal that neither throws nor consumes, - * so it cannot be told apart afterwards. Refuse it before that. */ + /* The one refusal that neither throws nor consumes: unrecognisable after. */ ZEND_ASSERT(nbufs > 0); if (UNEXPECTED(conn->write_timed_out)) { @@ -1474,9 +1468,7 @@ bool http_connection_send_strv_awaited(http_connection_t *conn, req->dispose(req); ok_total = ok && !had_exc && transferred == (ssize_t)total; } else { - /* Every refusal past the nbufs guard released the slots already; - * absorb the reactor's exception so one dropped connection does not - * reach the top level. */ + /* Past the guard every refusal has released the slots already. */ http_absorb_io_submission_exception(conn, __func__); } diff --git a/src/core/http_connection.h b/src/core/http_connection.h index 700d5fb4..2131c90f 100644 --- a/src/core/http_connection.h +++ b/src/core/http_connection.h @@ -462,11 +462,9 @@ bool http_connection_send_strv_owned(http_connection_t *conn, zend_string * const *bufs, unsigned nbufs); #ifdef ZEND_ASYNC_IO_WRITEV_AWAIT -/* Vectored variant the caller waits for. Each slot is an OWNED zend_string - * reference, consumed in every outcome — success, failure, refused submit, and - * a cancellation while parked. That last one is why it exists: the reactor - * holds the references until libuv is done, so nothing the caller built can be - * freed while a queued write still points at it. +/* Vectored variant the caller waits for. Each slot is an OWNED reference and is + * consumed in every outcome, a cancellation while parked included — the reactor + * keeps them until libuv is done, so a queued write never points at freed bytes. * Plaintext only — same TLS caveat as send_str_owned. Requires nbufs > 0. */ bool http_connection_send_strv_awaited(http_connection_t *conn, zend_string * const *bufs, unsigned nbufs); diff --git a/src/http1/http1_stream.c b/src/http1/http1_stream.c index 405bd93d..e4ed4991 100644 --- a/src/http1/http1_stream.c +++ b/src/http1/http1_stream.c @@ -105,20 +105,11 @@ static void h1_stream_headers_committed(http1_request_ctx_t *ctx) http_server_on_streaming_response_started(ctx->conn->counters); } -#ifdef ZEND_ASYNC_IO_WRITEV_AWAIT -static void h1_stream_headers_committed_if(http1_request_ctx_t *ctx, const bool sent) -{ - if (sent) { - h1_stream_headers_committed(ctx); - } -} -#endif #ifdef ZEND_ASYNC_IO_WRITEV_AWAIT -/* The CRLF every chunk frame ends with. Interned, because the vectored send - * takes a reference it will release: an interned string ignores both, so one - * literal serves every frame of every connection. A persistent-but-not-interned - * string would be decremented per frame and freed under the next one. */ +/* The CRLF every chunk frame ends with. Interned, because the send releases + * every slot and an interned string ignores that: a persistent one would be + * decremented per frame and freed under the next. */ static zend_string *h1_crlf_interned(void) { static zend_string *crlf = NULL; @@ -131,7 +122,6 @@ static zend_string *h1_crlf_interned(void) return crlf; } -static void h1_stream_headers_committed_if(http1_request_ctx_t *ctx, bool sent); #endif static bool h1_emit_headers_once(http1_request_ctx_t *ctx) @@ -252,14 +242,10 @@ static int h1_stream_append_chunk(void *opaque, zend_string *chunk, const size_t frame_len = head_len + (size_t)header_len + chunk_len + 2; #ifdef ZEND_ASYNC_IO_WRITEV_AWAIT - /* Plaintext: hand the pieces over as slots. One submit, one suspension, no - * copy of the body — and every piece belongs to the reactor until libuv is - * done with it, so a handler cancelled while parked cannot leave a queued - * write pointing at memory its C frame has released. - * - * TLS keeps the coalesced copy below: http_connection_send routes through - * tls_push, which copies into the BIO ring anyway, and a vectored write - * aimed straight at the socket would put plaintext on a TLS connection. */ + /* Plaintext: the pieces go over as slots the reactor owns — one submit, no + * copy of the body, and nothing a cancelled frame could leave dangling. + * TLS keeps the copy below: tls_push copies into the BIO ring anyway, and a + * vectored write at the socket would put plaintext on a TLS connection. */ #ifdef HAVE_OPENSSL const bool plaintext = conn->tls == NULL; #else @@ -286,7 +272,10 @@ static int h1_stream_append_chunk(void *opaque, zend_string *chunk, return HTTP_STREAM_APPEND_STREAM_DEAD; } - h1_stream_headers_committed_if(ctx, head_len != 0); + if (head_len != 0) { + h1_stream_headers_committed(ctx); + } + http_server_on_stream_send(conn->counters, chunk_len); return HTTP_STREAM_APPEND_OK; } From 06ac738987a0df40fbdf2f4d48bc92fd8945525c Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:03:36 +0000 Subject: [PATCH 3/6] fix(http1): the frame's CRLF is a fresh string, not a runtime-interned one (#179) zend_string_init_interned writes interned_strings_permanent, a process-wide table the engine treats as read-only after startup and guards with nothing. Calling it lazily from a worker thread is the shape that already produced a SEGV on the HTTP/2 header path. Two bytes per frame instead. --- src/http1/http1_stream.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/src/http1/http1_stream.c b/src/http1/http1_stream.c index e4ed4991..75c4bf7c 100644 --- a/src/http1/http1_stream.c +++ b/src/http1/http1_stream.c @@ -105,25 +105,6 @@ static void h1_stream_headers_committed(http1_request_ctx_t *ctx) http_server_on_streaming_response_started(ctx->conn->counters); } - -#ifdef ZEND_ASYNC_IO_WRITEV_AWAIT -/* The CRLF every chunk frame ends with. Interned, because the send releases - * every slot and an interned string ignores that: a persistent one would be - * decremented per frame and freed under the next. */ -static zend_string *h1_crlf_interned(void) -{ - static zend_string *crlf = NULL; - - if (UNEXPECTED(crlf == NULL)) { - crlf = zend_string_init_interned("\r\n", 2, 1); - ZEND_ASSERT(ZSTR_IS_INTERNED(crlf)); - } - - return crlf; -} - -#endif - static bool h1_emit_headers_once(http1_request_ctx_t *ctx) { zend_string *headers = h1_streaming_headers_build(ctx); @@ -263,7 +244,10 @@ static int h1_stream_append_chunk(void *opaque, zend_string *chunk, slots[n++] = zend_string_init(header, (size_t)header_len, 0); slots[n++] = chunk; /* the caller's ref, handed over */ - slots[n++] = h1_crlf_interned(); + /* Two bytes per frame rather than one shared literal: the send releases + * every slot, and interning at runtime would write the process-wide + * permanent table from a worker thread. */ + slots[n++] = zend_string_init("\r\n", 2, 0); const bool sent = http_connection_send_strv_awaited(conn, slots, n); From b37edf72bb5d54a27d985001efa0de9b874df3a6 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:19:49 +0000 Subject: [PATCH 4/6] fix(http1): gate on the API version, and close the two header sends too (#179) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The macro name says the header knows the flag; only the version says the reactor keeps it. A build pairing a new header with an old reactor ran the write as an ordinary fire-and-forget writev and waited for a notification nobody sends — a hang, or a dispose through a freed request once the write deadline closed the handle. h1_emit_headers_once and the empty-first-chunk branch had the same lifetime defect as the frame: send a header block the caller owns, then release it while libuv may still hold the pointer. Both go through one helper now, which hands the block to the reactor where it can. --- src/core/http_connection.c | 4 ++-- src/core/http_connection.h | 2 +- src/http1/http1_stream.c | 33 +++++++++++++++++++++++++-------- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/core/http_connection.c b/src/core/http_connection.c index 293a4113..e817af1a 100644 --- a/src/core/http_connection.c +++ b/src/core/http_connection.c @@ -1413,7 +1413,7 @@ bool http_connection_send_raw(http_connection_t *conn, } /* }}} */ -#ifdef ZEND_ASYNC_IO_WRITEV_AWAIT +#if defined(ZEND_ASYNC_API_VERSION_NUMBER) && ZEND_ASYNC_API_VERSION_NUMBER >= 0x001900 /* {{{ http_connection_send_strv_awaited * * Vectored plaintext send the caller waits for. Slots go out in array order; @@ -1483,7 +1483,7 @@ bool http_connection_send_strv_awaited(http_connection_t *conn, return ok_total; } /* }}} */ -#endif /* ZEND_ASYNC_IO_WRITEV_AWAIT */ +#endif /* async API >= 0.25 */ /* {{{ http_connection_send_str_owned * diff --git a/src/core/http_connection.h b/src/core/http_connection.h index 2131c90f..3a13d3d3 100644 --- a/src/core/http_connection.h +++ b/src/core/http_connection.h @@ -461,7 +461,7 @@ bool http_connection_send_batched_writev(http_connection_t *conn, bool http_connection_send_strv_owned(http_connection_t *conn, zend_string * const *bufs, unsigned nbufs); -#ifdef ZEND_ASYNC_IO_WRITEV_AWAIT +#if defined(ZEND_ASYNC_API_VERSION_NUMBER) && ZEND_ASYNC_API_VERSION_NUMBER >= 0x001900 /* Vectored variant the caller waits for. Each slot is an OWNED reference and is * consumed in every outcome, a cancellation while parked included — the reactor * keeps them until libuv is done, so a queued write never points at freed bytes. diff --git a/src/http1/http1_stream.c b/src/http1/http1_stream.c index 75c4bf7c..2baaa373 100644 --- a/src/http1/http1_stream.c +++ b/src/http1/http1_stream.c @@ -59,6 +59,28 @@ ZEND_STATIC_ASSERT(H1_CHUNK_COALESCE_MAX <= HTTP_TLS_PLAINTEXT_RING_BYTES, "a coalesced frame must fit one TLS plaintext ring cycle"); +/* Sends a header block the caller owns, consuming its reference. Where the + * reactor can take it over it does, so a cancellation cannot leave a queued + * write pointing at a released string; elsewhere the copy is unavoidable. */ +static bool h1_send_headers_owned(http_connection_t *conn, zend_string *headers) +{ +#if defined(ZEND_ASYNC_API_VERSION_NUMBER) && ZEND_ASYNC_API_VERSION_NUMBER >= 0x001900 +#ifdef HAVE_OPENSSL + const bool plaintext = conn->tls == NULL; +#else + const bool plaintext = true; +#endif + + if (plaintext) { + return http_connection_send_strv_awaited(conn, &headers, 1); + } +#endif + + const bool ok = http_connection_send(conn, ZSTR_VAL(headers), ZSTR_LEN(headers)); + zend_string_release(headers); + return ok; +} + /* The status line and headers of a streaming response, as bytes. Returns NULL * when the response is gone or formats to nothing; the caller owns the string. * Separate from the send so the first frame can carry the block with it. */ @@ -113,10 +135,7 @@ static bool h1_emit_headers_once(http1_request_ctx_t *ctx) return false; } - const bool ok = http_connection_send(ctx->conn, ZSTR_VAL(headers), - ZSTR_LEN(headers)); - zend_string_release(headers); - return ok; + return h1_send_headers_owned(ctx->conn, headers); } /* `nonblocking` is accepted and ignored: HTTP/1 keeps no queue of its own, so @@ -179,9 +198,7 @@ static int h1_stream_append_chunk(void *opaque, zend_string *chunk, zend_string_release(chunk); if (headers != NULL) { - const bool sent = http_connection_send(conn, ZSTR_VAL(headers), - ZSTR_LEN(headers)); - zend_string_release(headers); + const bool sent = h1_send_headers_owned(conn, headers); if (!sent) { ctx->stream_dead = true; @@ -222,7 +239,7 @@ static int h1_stream_append_chunk(void *opaque, zend_string *chunk, const size_t head_len = headers != NULL ? ZSTR_LEN(headers) : 0; const size_t frame_len = head_len + (size_t)header_len + chunk_len + 2; -#ifdef ZEND_ASYNC_IO_WRITEV_AWAIT +#if defined(ZEND_ASYNC_API_VERSION_NUMBER) && ZEND_ASYNC_API_VERSION_NUMBER >= 0x001900 /* Plaintext: the pieces go over as slots the reactor owns — one submit, no * copy of the body, and nothing a cancelled frame could leave dangling. * TLS keeps the copy below: tls_push copies into the BIO ring anyway, and a From 9f7492352d55c92b115252ce0e74f67a71e3dc9b Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:20:53 +0000 Subject: [PATCH 5/6] test(http1): the cancel test now reads the frames and the tail (#179) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It asserted only the exception, which the path it replaced produced just as readily — every revert in the diff passed it. It now checks the opening frames byte for byte, so a frame built in the wrong order or from a released slot fails, and checks that no terminator follows the cancellation, which is the invariant that keeps a cut frame from desynchronising the next request. The write timeout is one second so a lost wake fails instead of hanging. --- .../030-h1-cancel-while-parked-in-write.phpt | 63 ++++++++++++++++--- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/tests/phpt/server/h1/030-h1-cancel-while-parked-in-write.phpt b/tests/phpt/server/h1/030-h1-cancel-while-parked-in-write.phpt index 47218a95..41f79f39 100644 --- a/tests/phpt/server/h1/030-h1-cancel-while-parked-in-write.phpt +++ b/tests/phpt/server/h1/030-h1-cancel-while-parked-in-write.phpt @@ -19,10 +19,14 @@ sockets * wins the race. And the response must be streaming, because that is the only * path where the handler owns the write. * - * What it asserts is the contract, not the defect: the cancellation reaches the - * handler as HttpException 499, start() returns, and the process ends. A hang - * here means the cancel never arrived; a crash means the frame outlived - * something it pointed at. */ + * Three assertions, and each one pins something different. The frames read + * before the cancel are checked byte for byte, which is what catches a frame + * assembled in the wrong order or with a piece already released. The tail read + * after it must carry no terminator: sealing a frame the cancellation cut in + * half is what desynchronises the next request on a kept-alive connection. And + * the cancellation itself must arrive as HttpException 499 with isWritable() + * false afterwards. A write timeout of one second turns a lost wake into a + * bounded failure rather than a suite that hangs. */ use TrueAsync\HttpServer; use TrueAsync\HttpServerConfig; @@ -35,7 +39,7 @@ require_once __DIR__ . '/../_free_port.inc'; $port = tas_free_port(); $server = new HttpServer((new HttpServerConfig()) ->addListener('127.0.0.1', $port) - ->setReadTimeout(30)->setWriteTimeout(30) + ->setReadTimeout(30)->setWriteTimeout(1) ->setShutdownTimeout(0)); /* no grace window: cancel at once */ $seen = []; @@ -45,8 +49,8 @@ $server->addHttpHandler(function ($req, $res) use (&$seen) { try { /* Far more than any socket buffer holds; parks after the first chunks. */ - for ($i = 0; $i < 5000; $i++) { - $res->write(str_repeat('x', 20000)); + for ($i = 0; $i < 20000; $i++) { + $res->write(str_repeat('x', 2000)); } $res->end(); $seen['outcome'] = 'finished'; @@ -65,11 +69,52 @@ $cli = spawn(function () use ($port, $server) { fwrite($fp, "GET /stream HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); - /* Read nothing at all: the handler parks inside the write. */ + /* Take the opening frames, then stop reading so the handler parks. */ + delay(200); + stream_set_blocking($fp, false); + $head = ''; + + for ($i = 0; $i < 40 && strlen($head) < 16384; $i++) { + $c = fread($fp, 8192); + if ($c !== false) $head .= $c; + usleep(5000); + } + delay(500); $server->stop(); delay(500); + + /* Whatever the cancelled response still had to say. */ + $tail = ''; + + for ($i = 0; $i < 60; $i++) { + $c = fread($fp, 65536); + if ($c !== false) $tail .= $c; + usleep(5000); + } + fclose($fp); + + [, $rest] = explode("\r\n\r\n", $head, 2); + $sizes = []; + $off = 0; + + while (count($sizes) < 3) { + $eol = strpos($rest, "\r\n", $off); + if ($eol === false) break; + $len = hexdec(substr($rest, $off, $eol - $off)); + $off = $eol + 2; + if ($len === 0 || $off + $len + 2 > strlen($rest)) break; + /* The frame must be exactly what the handler wrote, and end where the + * size line said it would — a released or misplaced slot shows here. */ + if (substr($rest, $off, $len) !== str_repeat('x', $len)) break; + if (substr($rest, $off + $len, 2) !== "\r\n") break; + $sizes[] = $len; + $off += $len + 2; + } + + echo "frames=", implode(',', $sizes), "\n"; + echo "terminator_after_cancel=", (int)(strpos($tail, "0\r\n\r\n") !== false), "\n"; }); $server->start(); @@ -79,6 +124,8 @@ foreach ($seen as $k => $v) echo "$k = ", var_export($v, true), "\n"; echo "done\n"; ?> --EXPECT-- +frames=2000,2000,2000 +terminator_after_cancel=0 outcome = 'TrueAsync\\HttpException:499' writable_after = false done From f8c0f39214c5d37bab84737bae60df3d579d7850 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Thu, 20 Aug 2026 17:08:24 +0000 Subject: [PATCH 6/6] fix(http1): dispose must not seal a frame a cancellation cut in half (#179) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mark_ended refuses the terminator when the stream is dead, and the dispose path skips mark_ended entirely: it emitted 0\r\n\r\n on its own and left keep_alive true. The peer then reads the terminator as the first bytes of the chunk the orphaned size line promised, and the next request on the connection desyncs — the defect 4c7824c closed in one place and not the other. Caught by 030 on macOS, where the write still lands; on Linux the socket is gone by then and the same code passes. --- src/core/http_connection.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/core/http_connection.c b/src/core/http_connection.c index e817af1a..f0a26fac 100644 --- a/src/core/http_connection.c +++ b/src/core/http_connection.c @@ -2853,7 +2853,15 @@ void http_handler_coroutine_dispose(zend_coroutine_t *coroutine) conn->state = CONN_STATE_SENDING; if (http_response_is_streaming(Z_OBJ(ctx->response_zv))) { - if (!http_response_is_closed(Z_OBJ(ctx->response_zv))) { + if (UNEXPECTED(ctx->stream_dead)) { + /* A cancellation can cut a frame in half, and the peer has been + * promised the bytes its size line named. Sealing that with the + * terminator would say the body ended cleanly and hand the + * connection on, and the peer would read the terminator as the + * first bytes of what it is still waiting for. mark_ended refuses + * the same way; this is the path that skips mark_ended. */ + conn->keep_alive = 0; + } else if (!http_response_is_closed(Z_OBJ(ctx->response_zv))) { /* Handler fell through without end() — emit the terminator. */ (void)http_connection_send(conn, "0\r\n\r\n", 5); }