fix(core): a half-closed peer keeps its response, and a gone one is still caught (#249)

CHANGELOG.md

@@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **A peer that shut its write half down lost the rest of its response (#249).** Every terminating read latched the connection unwritable, and a clean EOF is what a half-close produces — so a handler streaming to a peer that had merely finished sending had its output dropped: over HTTP/1 the client got 110 bytes and no chunked terminator where 4 MiB were owed, over HTTP/2 between 65536 and 393216 DATA bytes of 4194304 and no END_STREAM. The read side now latches on a read error alone. The verdict a fire-and-forget write could not deliver comes from the handle instead: the reactor sets `ZEND_ASYNC_IO_WRITE_FAILED` on a failed write (TrueAsync ABI 0.26.0) and every write completion reads it, which keeps `trySseEvent()` and `tryWriteMessage()` answering 499 to a peer that is genuinely gone — the RST is consumed by the write, not by the read, so the read behind it returns a clean EOF and told nothing. Evidence: `h1/057`, `h2/064`, both failing against `main`; `h1/032` holds the other half.
 - **HTTP/3 dropped a streamed response's trailers when the handler called `end()` (#247).** `end()` reaches `h3_stream_mark_ended`, which resumes the stream and drains, so the data reader can reach EOF inside that call; the trailers were captured afterwards, in the dispose, and the fin had already gone. A stream the dispose ended kept them, which is why the shape went unnoticed. Measured with the aioquic client, one trailer, four handler shapes: `end()` after the write lost it whether the trailer was set before the write or after, while a dispose-ended stream, a buffered response and every shape under `TRUE_ASYNC_SERVER_REACTOR_POOL=1` kept it; HTTP/2 kept all four. The capture now runs before the latch in `h3_stream_mark_ended`, the order `h3_stream_finish_streaming` already used for gRPC, and it is idempotent so the dispose-side call stays correct. Evidence: `h3/067`.
 - **HTTP/3 stopped emitting response headers at 256 and dropped the Content-Length with them (#247).** The flatten loop left through a `goto` on the 257th field and reported nothing, so the fields the hash order had put last were gone — `content-length` among them — while the body went out in full, unframed. Measured with 300 headers and a 512-byte body: the direct path delivered 254 headers and no `content-length`, against 300 and the `content-length` from both the reactor pool and HTTP/2. The cap is removed rather than mirrored: it arrived with the initial import behind no issue, named its threat as "a server-side accident" rather than a peer, and bounded 10 KB of `nghttp3_nv` copied out of a `HashTable` that already holds the same strings at several times the cost. HTTP/3's real limit is the peer's `SETTINGS_MAX_FIELD_SECTION_SIZE`, which is a negotiated byte count and not this. Evidence: `h3/068`.
 - **A handler killed by a bailout answered 200 over the body it had half-built (#244).** A `zend_bailout` longjmps out of the handler without unwinding it, so the response object keeps the status and the part-written body it held at that instant. HTTP/1 replaced those with a 500; HTTP/2, HTTP/3 and the reactor pool derived a status from `coroutine->exception`, which a bailout leaves NULL, and committed what the handler had left. Measured with a handler that sets a 200 and the body `half-built` and then exhausts an 8 MB `memory_limit`: HTTP/1 answered `500` with 21 bytes, while HTTP/2, HTTP/3 and HTTP/3 under `TRUE_ASYNC_SERVER_REACTOR_POOL=1` each answered `200` with the 10 bytes, `content-length` included. The same answers came back from a stack-overflow bailout under a 512 MB limit, so it is the longjmp rather than the allocation failure. A bailed-out gRPC call was reported as `grpc-status: 0` and is now 13. Every transport replaces an uncommitted response through one predicate, `http_response_reset_after_bailout`, which is the block HTTP/1 already carried. Evidence: `core/069`, `grpc/019`, `h3/064`, `h3/065`.

dev/PLAN.md

@@ -946,3 +946,79 @@ the suite had, since every H3 test reads its response to the end.
 
   Evidence: `h3/067`, `h3/068`, both failing against `main`. 485 phpt, 461 passed,
   0 failed, 0 warned; `ctest` 16 of 16.
+
+- [x] **`h2/060` flakes because php-async builds a PHP object after the object
+  store is gone.** The test's own assertions pass every time; the process
+  segfaults afterwards, which `run-tests` prints as `Termsig=0` under correct
+  output. Two failures in 17 full `-j4` sweeps, four in 30 standalone runs.
+
+  Cause, from a backtrace of mine rather than a reading: `zend_deactivate` runs
+  `shutdown_executor()` and only then `ZEND_ASYNC_ENGINE_SHUTDOWN()` — php-src
+  states the contract at that line, "All objects are destroyed — safe to shut
+  down the reactor", and `zend_shutdown_executor_values` has already set
+  `EG(active) = 0`, "No PHP callback functions should be called after this
+  point". `libuv_reactor_shutdown` then turns the loop to finish cancelling
+  work, a connection this server closed earlier finishes closing there, and
+  libuv flushes its queued write with `UV_ECANCELED`. `io_pipe_writev_cb`
+  branches on `status == 0` alone and sends everything else to
+  `async_new_exception` → `object_init_ex` → `zend_objects_store_put`, writing
+  into an `object_buckets` that is NULL.
+
+  The handle is ours (`stream->type == UV_TCP`,
+  `free_cb == http_send_batched_writev_completion_cb`, `uv_flags == 0`, so no
+  awaiter), but no frame of this repository is on the stack and the contract
+  broken is php-async's. The fix belongs there and Edmond agreed to it: a check
+  in `async_new_exception`, plus four sites that use its result without a NULL
+  check — `libuv_reactor.c:2204`, `:2870`, `:3126`, `:6286`. The audit of all 27
+  call sites is done; the other 23 carry NULL.
+
+  No `.phpt` in php-async can reproduce it: the queue of writes on a stream
+  handle exists only through `ZEND_ASYNC_IO_WRITEV_EX`, whose one consumer is
+  this server, out of that tree. Measured, not reasoned — a temporary hook in
+  `plain_wrapper.c` that leaves a write unawaited fires and leaks the request but
+  exits 0, because a file write is waited out by the drain rather than cancelled;
+  and `plain_wrapper.c:553` is the only caller of the write API in all of php-src,
+  sockets not using it at all. So php-async gets a test-only entry point behind a
+  build flag, agreed with Edmond, and the deterministic test rides on that.
+
+  Done in php-async (issue #264, PR 265): `async_new_exception` answers NULL
+  while `EG(active)` is 0, and the four call sites take that NULL. The entry
+  point is a write queued at reactor shutdown when `ASYNC_FUZZ_CANCELLED_WRITE=1`
+  is set, compiled in only with `--enable-async-fuzz`, and
+  `tests/cleanup/005-write_cancelled_at_reactor_shutdown.phpt` rides on it:
+  without the guard it prints its expected output and segfaults, with it 5 of 5
+  runs pass. Against a PHP carrying the change, `h2/060` failed 0 of 30
+  standalone runs where it had failed 4, and this suite ran 369 tests to 349
+  passed, 20 skipped, 0 failed on that binary. The change reaches this
+  repository only through a php-src build that carries it; `/usr/local/bin/php`,
+  which the suite runs against by default, is older.
+
+## A half-closed peer lost the rest of its response (#249)
+
+- [x] **The read side stopped standing in for the write side.** Found while
+  building the reproduction #225 still owed: a peer that calls
+  `shutdown(SHUT_WR)` and goes on reading was answered with 110 bytes and no
+  chunked terminator where 4 MiB were owed, and over HTTP/2 with 65536 to 393216
+  DATA bytes of 4194304 and no END_STREAM. Every terminating read latched
+  `conn->write_failed`, and a clean EOF is what a half-close produces.
+
+  The one-line experiment named the cause: `conn->write_failed = err` and the
+  whole body arrived. It is not the whole fix, and `h1/032` said so — with that
+  alone the handler ran all 100000 iterations of 4 KiB against a peer that had
+  sent an RST and never saw the 499 its contract promises. The kernel hands
+  `so_error` to whichever syscall asks first, and with a saturated queue that is
+  the write: on bare sockets after an RST, `write -> ECONNRESET`,
+  `write -> EPIPE`, `read -> b''`.
+
+  So the write reports for itself. `ZEND_ASYNC_IO_WRITE_FAILED` is set by the
+  reactor on a failed write (true-async/php-src#27, ABI 0.26.0;
+  true-async/php-async#266) and read by every write completion here, which also
+  releases the outbound tail rather than chaining another refused write. The
+  read side latches on a read error alone, under the version guard the file
+  already uses.
+
+  Evidence: `h1/057` and `h2/064`, both failing against `main`; `h1/032` holds
+  the other half. 351 of 371 phpt (20 skipped, 0 failed), `ctest` 16 of 16.
+
+  Two fixes recorded in the CHANGELOG were carried without a test and still are:
+  #224 and #225. The reproduction for #225 is what turned into this step.

h2load1.log

@@ -0,0 +1,40 @@
+starting benchmark...
+spawning thread #0: 16 total client(s). Timing-based test with 0s of warm-up time and 22s of main duration for measurements.
+spawning thread #1: 16 total client(s). Timing-based test with 0s of warm-up time and 22s of main duration for measurements.
+Warm-up started for thread #1.
+Warm-up started for thread #0.
+progress: 6% of clients started
+progress: 12% of clients started
+progress: 18% of clients started
+progress: 25% of clients started
+progress: 31% of clients started
+progress: 37% of clients started
+progress: 43% of clients started
+progress: 50% of clients started
+progress: 56% of clients started
+progress: 62% of clients started
+progress: 68% of clients started
+progress: 75% of clients started
+Warm-up phase is over for thread #1.
+Main benchmark duration is started for thread #1.
+progress: 81% of clients started
+progress: 87% of clients started
+progress: 93% of clients started
+progress: 100% of clients started
+Warm-up phase is over for thread #0.
+Main benchmark duration is started for thread #0.
+Application protocol: http/1.1
+Main benchmark duration is over for thread #0. Stopping all clients.
+Stopped all clients for thread #0
+Main benchmark duration is over for thread #1. Stopping all clients.
+Stopped all clients for thread #1
+
+finished in 22.02s, 2297.86 req/s, 179.52KB/s
+requests: 50553 total, 50585 started, 50553 done, 50553 succeeded, 0 failed, 0 errored, 0 timeout
+status codes: 50553 2xx, 0 3xx, 0 4xx, 0 5xx
+traffic: 3.86MB (4044240) total, 2.31MB (2426544) headers (space savings 0.00%), 246.84KB (252765) data
+                     min         max         mean         sd        +/- sd
+time for request:       47us      2.51ms       290us       126us    81.52%
+time for connect:        3us        94us        44us        27us    56.25%
+time to 1st byte:      478us       680us       560us        52us    75.00%
+req/s           :    3099.72     4655.23     3699.53      445.84    65.63%

h2load2.log

@@ -0,0 +1,52 @@
+starting benchmark...
+spawning thread #0: 16 total client(s). Timing-based test with 0s of warm-up time and 12s of main duration for measurements.
+spawning thread #1: 16 total client(s). Timing-based test with 0s of warm-up time and 12s of main duration for measurements.
+spawning thread #2: 16 total client(s). Timing-based test with 0s of warm-up time and 12s of main duration for measurements.
+spawning thread #3: 16 total client(s). Timing-based test with 0s of warm-up time and 12s of main duration for measurements.
+Warm-up started for thread #1.
+Warm-up started for thread #2.
+Warm-up started for thread #0.
+progress: 6% of clients started
+progress: 12% of clients started
+Warm-up started for thread #3.
+progress: 18% of clients started
+progress: 25% of clients started
+progress: 31% of clients started
+progress: 37% of clients started
+progress: 43% of clients started
+progress: 50% of clients started
+progress: 56% of clients started
+progress: 62% of clients started
+progress: 68% of clients started
+progress: 75% of clients started
+progress: 81% of clients started
+progress: 87% of clients started
+Warm-up phase is over for thread #1.
+Main benchmark duration is started for thread #1.
+progress: 93% of clients started
+progress: 100% of clients started
+Warm-up phase is over for thread #0.
+Main benchmark duration is started for thread #0.
+Application protocol: http/1.1
+Warm-up phase is over for thread #2.
+Main benchmark duration is started for thread #2.
+Warm-up phase is over for thread #3.
+Main benchmark duration is started for thread #3.
+Main benchmark duration is over for thread #1. Stopping all clients.
+Stopped all clients for thread #1
+Main benchmark duration is over for thread #0. Stopping all clients.
+Stopped all clients for thread #0
+Main benchmark duration is over for thread #2. Stopping all clients.
+Stopped all clients for thread #2
+Main benchmark duration is over for thread #3. Stopping all clients.
+Stopped all clients for thread #3
+
+finished in 12.04s, 16677.92 req/s, 1.27MB/s
+requests: 200135 total, 200199 started, 200135 done, 200135 succeeded, 0 failed, 0 errored, 0 timeout
+status codes: 200135 2xx, 0 3xx, 0 4xx, 0 5xx
+traffic: 15.27MB (16010800) total, 9.16MB (9606480) headers (space savings 0.00%), 977.22KB (1000675) data
+                     min         max         mean         sd        +/- sd
+time for request:       25us     30.45ms       325us       448us    98.83%
+time for connect:        1us       120us        37us        30us    78.13%
+time to 1st byte:      374us     30.45ms     21.13ms     11.49ms    75.00%
+req/s           :    2266.44     5665.39     3068.72      983.93    87.50%

h2load4.log

@@ -0,0 +1,32 @@
+starting benchmark...
+spawning thread #0: 8 total client(s). Timing-based test with 0s of warm-up time and 14s of main duration for measurements.
+spawning thread #1: 8 total client(s). Timing-based test with 0s of warm-up time and 14s of main duration for measurements.
+Warm-up started for thread #0.
+Warm-up started for thread #1.
+progress: 12% of clients started
+progress: 25% of clients started
+progress: 37% of clients started
+progress: 50% of clients started
+progress: 62% of clients started
+progress: 75% of clients started
+progress: 87% of clients started
+progress: 100% of clients started
+Warm-up phase is over for thread #0.
+Main benchmark duration is started for thread #0.
+Application protocol: http/1.1
+Warm-up phase is over for thread #1.
+Main benchmark duration is started for thread #1.
+Main benchmark duration is over for thread #1. Stopping all clients.
+Stopped all clients for thread #1
+Main benchmark duration is over for thread #0. Stopping all clients.
+Stopped all clients for thread #0
+
+finished in 14.02s, 1.14 req/s, 91B/s
+requests: 16 total, 32 started, 16 done, 16 succeeded, 0 failed, 0 errored, 0 timeout
+status codes: 16 2xx, 0 3xx, 0 4xx, 0 5xx
+traffic: 1.25KB (1280) total, 768B (768) headers (space savings 0.00%), 80B (80) data
+                     min         max         mean         sd        +/- sd
+time for request:    30.48ms     31.17ms     30.79ms       315us    62.50%
+time for connect:        2us       102us        49us        22us    75.00%
+time to 1st byte:    30.52ms     31.26ms     30.84ms       319us    56.25%
+req/s           :      31.62       32.19       31.93        0.22    56.25%

src/core/http_connection.c

@@ -1199,9 +1199,13 @@ static void http_connection_read_callback_fn(
             ws_session_mark_peer_closed(ws_strategy_get_session(conn->strategy));
         }
 #endif
-        /* Where a queued write's failure becomes visible: libuv reports it at
-         * completion, and a fire-and-forget completion carries no status. */
-        conn->write_failed = true;
+        /* A read error stands for a write failure the completion could not
+         * report; a clean EOF does not — the peer that shut its write half down
+         * goes on reading, and this flag means "output can no longer reach the
+         * peer". The write side reports for itself, so this only latches. */
+        if (err) {
+            conn->write_failed = true;
+        }
         async_plain_event_fire(conn->out_idle_event);
         async_plain_event_fire(conn->out_drain_event);
 
@@ -1874,6 +1878,25 @@ static bool out_wait_for_tail(http_connection_t *conn)
     return true;
 }
 
+/* Read before the tail is queued: the peer that refused these bytes will refuse
+ * the next ones, and the tail is released here so nothing chains another
+ * refused write. Contract at the declaration. */
+void http_connection_absorb_write_verdict(http_connection_t *conn, const zend_async_io_t *io)
+{
+    if (EXPECTED((io->state & ZEND_ASYNC_IO_WRITE_FAILED) == 0)) {
+        return;
+    }
+
+    conn->write_failed = true;
+
+    if (conn->out_pending_len > 0) {
+        efree(conn->out_pending_buf);
+        conn->out_pending_buf = NULL;
+        conn->out_pending_len = 0;
+        conn->out_pending_cap = 0;
+    }
+}
+
 static void http_send_batched_completion_cb(void *data, zend_async_io_t *io)
 {
     efree(data);
@@ -1884,6 +1907,8 @@ static void http_send_batched_completion_cb(void *data, zend_async_io_t *io)
 
     http_connection_t *conn = (http_connection_t *)io->user_data;
 
+    http_connection_absorb_write_verdict(conn, io);
+
     if (conn->out_pending_len > 0) {
         char  *next_buf = conn->out_pending_buf;
         size_t next_len = conn->out_pending_len;
@@ -1947,6 +1972,8 @@ static void http_send_batched_zstr_completion_cb(void *data, zend_async_io_t *io
 
     http_connection_t *conn = (http_connection_t *)io->user_data;
 
+    http_connection_absorb_write_verdict(conn, io);
+
     if (h1_batched_drain_pending(conn)) {
         return;
     }
@@ -2036,6 +2063,8 @@ static void http_send_batched_writev_completion_cb(void *data, zend_async_io_t *
 
     http_connection_t *conn = (http_connection_t *)data;
 
+    http_connection_absorb_write_verdict(conn, io);
+
     /* User release first; pulled to a local so a re-entrant emit sees cleared slot. */
     zend_async_io_write_free_cb_t user_cb = conn->out_writev_user_cb;
     void *user_data = conn->out_writev_user_data;

src/core/http_connection.h

@@ -432,6 +432,12 @@ uint16_t     http_connection_remote_port(const http_connection_t *conn);
  * the connection is not idle or a teardown microtask is already queued. */
 void http_connection_destroy_if_idle_deferred(http_connection_t *conn);
 
+/* Take the reactor's verdict on a write that has just completed, and release
+ * the outbound tail if it failed. Every write completion calls this: a write
+ * submitted without an awaiter gets no status of its own, so the failure is
+ * reported on the handle instead. */
+void http_connection_absorb_write_verdict(http_connection_t *conn, const zend_async_io_t *io);
+
 /* Connection processing */
 bool http_connection_send(http_connection_t *conn, const char *data, size_t len);
 bool http_connection_send_error(http_connection_t *conn, int status_code, const char *message);

src/core/http_connection_tls.c

@@ -447,6 +447,8 @@ static void tls_cipher_completion(void *data, zend_async_io_t *io)
 
     http_connection_t *conn = (http_connection_t *)io->user_data;
 
+    http_connection_absorb_write_verdict(conn, io);
+
     if (UNEXPECTED(conn->tls == NULL)) {
         return;
     }