Skip to content

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

Merged
EdmondDantes merged 3 commits into
mainfrom
249-half-closed-peer-loses-response
Aug 23, 2026
Merged

EdmondDantes merged 3 commits into
mainfrom
249-half-closed-peer-loses-response

Conversation

@EdmondDantes

Copy link
Copy Markdown
Contributor

Closes #249. Needs true-async/php-src#27 and true-async/php-async#266, both merged — the reactor's report is what makes the second half of this work.

What was wrong

http_connection_read_callback_fn latched conn->write_failed on every terminating read, and a clean EOF is what a half-close produces. The peer had only finished sending; every reader of that flag takes it as "output can no longer reach the peer", so a streaming handler lost the rest of its response.

Measured with a 4 MiB body written in 64 KiB chunks, the client sending one request, calling shutdown(SHUT_WR), then reading to EOF:

half-closed control
HTTP/1 110 wire bytes, headers only, no terminator 4194995, terminator
HTTP/2 65536–393216 DATA bytes of 4194304, no END_STREAM 4194304, END_STREAM

Why the read side alone cannot decide it

The kernel hands so_error to whichever syscall asks first. With a saturated outbound queue that is the write, so the read behind it returns a clean EOF and reports nothing. On bare sockets after an RST: write -> ECONNRESET, write -> EPIPE, read -> b''.

That is why dropping the latch on its own is not the fix. h1/032-h1-try-twins-peer-gone proves it: with the read-side change alone the handler ran all 100000 iterations of 4 KiB against a peer that had sent an RST and never saw an error, where the contract is HttpException 499.

What lands here

The read side latches on a read error alone. The verdict a fire-and-forget write could not deliver now comes from the handle: ZEND_ASYNC_IO_WRITE_FAILED, set by the reactor, read by http_connection_absorb_write_verdict from all four write completions — the three batched ones and the TLS cipher completion. A latched connection releases its outbound tail instead of chaining another refused write.

Guarded on ZEND_ASYNC_API_VERSION_NUMBER >= 0x001A00, the form the file already uses: built against an older reactor there is no other report to use, so the previous behaviour compiles in.

Evidence

h1/057 and h2/064 are new and fail against main: HTTP/1 gets 1 response, 0 terminators and 0 body bytes of 2097152; HTTP/2 gets 393216 of 1048576 with no END_STREAM. h1/032 holds the other half and fails on the read-side change alone. Full suite 351 of 351, ctest 16 of 16, on a PHP built from php-src true-async at ABI 0.26.0.

…till caught (#249)

Every terminating read latched conn->write_failed, and a clean EOF is what a
half-close produces: the peer had only finished sending, but every reader of
that flag takes it as "output can no longer reach the peer". A streaming
handler lost the rest of its response — 110 bytes and no terminator over
HTTP/1 where 4 MiB were owed, 65536 to 393216 DATA bytes of 4194304 and no
END_STREAM over HTTP/2.

The read side now latches on a read error alone. What a fire-and-forget write
could not report comes from the handle: the reactor sets
ZEND_ASYNC_IO_WRITE_FAILED on a failed write, and every write completion takes
that verdict and releases the outbound tail. Without it a peer that is truly
gone would go unnoticed — the kernel hands so_error to whichever syscall asks
first, and with a saturated queue that is the write, so the read behind it
returns a clean EOF and says nothing. Measured on bare sockets after RST:
write -> ECONNRESET, write -> EPIPE, read -> b''.

Needs TrueAsync ABI 0.26.0; against an older reactor the previous behaviour is
compiled in, since there is no other report to use.

Evidence: h1/057 and h2/064 fail against main, h1/032 holds the other half and
would fail on the read-side change alone. 351 of 351 phpt, ctest 16 of 16.
@EdmondDantes
EdmondDantes force-pushed the 249-half-closed-peer-loses-response branch from e5082ae to ce6062c Compare August 23, 2026 17:30
@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Coverage

Total lines: 83.06% → 83.00% (-0.06 pp)

File Baseline Current Δ Touched
src/core/http_connection.c 81.26% 81.44% +0.18 pp
src/core/http_connection.h 81.82% 81.82% +0.00 pp
src/core/http_connection_tls.c 73.48% 73.55% +0.07 pp
src/http1/http_parser.c 84.43% 84.05% -0.38 pp
src/http3/http3_callbacks.c 83.37% 83.05% -0.32 pp
src/http3/http3_listener.c 77.36% 76.91% -0.45 pp
src/http3/http3_packet.c 90.43% 85.22% -5.22 pp
src/http_server_config.c 92.48% 92.42% -0.06 pp
src/websocket/php_websocket.c 78.44% 78.61% +0.17 pp
src/websocket/ws_session.c 90.04% 89.48% -0.55 pp

@EdmondDantes
EdmondDantes merged commit 7ffd726 into main Aug 23, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A half-closed peer loses the rest of its response

1 participant