fix(core): a half-closed peer keeps its response, and a gone one is still caught (#249) - #250
Merged
Merged
Conversation
…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
force-pushed
the
249-half-closed-peer-loses-response
branch
from
August 23, 2026 17:30
e5082ae to
ce6062c
Compare
Contributor
CoverageTotal lines: 83.06% → 83.00% (-0.06 pp)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_fnlatchedconn->write_failedon 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:Why the read side alone cannot decide it
The kernel hands
so_errorto 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-goneproves 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 isHttpException499.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 byhttp_connection_absorb_write_verdictfrom 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/057andh2/064are new and fail againstmain: HTTP/1 gets 1 response, 0 terminators and 0 body bytes of 2097152; HTTP/2 gets 393216 of 1048576 with no END_STREAM.h1/032holds the other half and fails on the read-side change alone. Full suite 351 of 351,ctest16 of 16, on a PHP built from php-srctrue-asyncat ABI 0.26.0.