Shut the client read side down per transaction, not per connection - #13523
Conversation
HttpSM::state_watch_for_client_abort reached past the transaction to _ua.get_txn()->get_netvc() to half close the client read side on an early EOS. For HTTP/2 and HTTP/3 that NetVConnection is shared by every stream on the connection, so a single aborted stream stopped the session from reading frames for all of the others. Route the shutdown through the transaction instead: Http2Stream and HQTransaction already implement do_io_shutdown() as a deliberate no-op for exactly this reason, and HTTP/1.x is unaffected because ProxyTransaction forwards to the session's NetVConnection. This is the remaining half of apache#12529. That change was written to address two regressions from apache#12502, the second being "HTTP/2 connection is closed if a stream is reset even if other streams are alive", with the stated approach of calling ProxyTransaction::do_io_shutdown() instead of NetVConnection::do_io_shutdown(). It converted the two branches it added but left the pre-existing IO_SHUTDOWN_READ branch calling the NetVConnection directly, so the connection-wide shutdown survived for the case where the tunnel still has a consumer besides the client. A response transform reaches that case readily: the transform stage runs the whole body before anything is written back, so the stream has no write of its own and a client reset arrives as an EOS on the stream's read VIO. Clearing the session's read VIO buffer is not inert, because Http2CommonSession still holds that VIO and re-enables it every 128 frames through HTTP2_SESSION_EVENT_REENABLE. A release build then takes the ntodo() <= 0 path in net_read_io and the connection stalls silently, failing every in-flight stream; a debug build aborts on SSLNetVConnection's `ink_assert(buf.writer())`, the assertion reported in apache#9448. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
bneradt
added
HTTP/2
Bug
Regression
AuTest
Backport
|
I traced every claim in the description and they all hold. Recording the verification since the one-line change conceals a fair amount of reasoning. The two no-op overrides exist, which is what makes the routing work. HTTP/1.x is genuinely unaffected. Retaining the The failure mode is exactly as described.
And the stall is permanent in the way you describe: The #12529 history checks out. Its diff converted the branch it added ( The fix is also complete. Nice choice using a response transform to reach the case: since the transform stage buffers the whole body before anything is written back, the stream has no write of its own and the client reset arrives as an EOS on the stream's read VIO, which is precisely the Backport note. Given the Nothing blocking from me. |
…13523) HttpSM::state_watch_for_client_abort reached past the transaction to _ua.get_txn()->get_netvc() to half close the client read side on an early EOS. For HTTP/2 and HTTP/3 that NetVConnection is shared by every stream on the connection, so a single aborted stream stopped the session from reading frames for all of the others. Route the shutdown through the transaction instead: Http2Stream and HQTransaction already implement do_io_shutdown() as a deliberate no-op for exactly this reason, and HTTP/1.x is unaffected because ProxyTransaction forwards to the session's NetVConnection. This is the remaining half of #12529. That change was written to address two regressions from #12502, the second being "HTTP/2 connection is closed if a stream is reset even if other streams are alive", with the stated approach of calling ProxyTransaction::do_io_shutdown() instead of NetVConnection::do_io_shutdown(). It converted the two branches it added but left the pre-existing IO_SHUTDOWN_READ branch calling the NetVConnection directly, so the connection-wide shutdown survived for the case where the tunnel still has a consumer besides the client. A response transform reaches that case readily: the transform stage runs the whole body before anything is written back, so the stream has no write of its own and a client reset arrives as an EOS on the stream's read VIO. Clearing the session's read VIO buffer is not inert, because Http2CommonSession still holds that VIO and re-enables it every 128 frames through HTTP2_SESSION_EVENT_REENABLE. A release build then takes the ntodo() <= 0 path in net_read_io and the connection stalls silently, failing every in-flight stream; a debug build aborts on SSLNetVConnection's `ink_assert(buf.writer())`, the assertion reported in #9448. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 31eb68f)
|
Cherry-picked to the 10.2.x branch as 34a0030 for the 10.2.0 release. |
Three late bug fixes on 10.2.x. All are fixes with no new configuration, metrics or API surface, so only the changelog and the commit/PR counts change.
* Add 10.2.0 changelog and release notes Generate CHANGELOG-10.2.0 from the 10.2.0 milestone and document the release in whats-new and upgrading. The connect retry change (#13102) is called out as a necessary incompatible change, since the retry limits were not previously applied according to origin state. * Address review: fix PR count and token_key markup The PR count was 655 before five stale milestone entries were dropped; the changelog has 650. Use :ts:cv: for proxy.config.quic.server.token_key.filename, which is documented on 10.2.x even though it is absent from master, where it was first checked. * Add late 10.2.x additions to changelog and release notes Picks up #13328 (shared-memory cache directory for fast restart) and #13418 (traffic_ctl cache clear). The shm directory gets its own section since it is a new opt-in feature with four new records and a traffic_ctl subcommand. * Add July 2026 security fixes to changelog and release notes The Release 2 security bundle (#13452) landed directly on 10.2.x without public PRs, so those commits never appear in a milestone. Source them from the commit range with the changelog tool's git-range mode and append them as bare subjects, matching how CHANGELOG-10.1.4 lists them. Link the advisory from whats-new for the CVE mapping. * Add #13352, #13517 and #13523 to the changelog Three late bug fixes on 10.2.x. All are fixes with no new configuration, metrics or API surface, so only the changelog and the commit/PR counts change.
HttpSM::state_watch_for_client_abort reached past the transaction to
_ua.get_txn()->get_netvc() to half close the client read side on an early
EOS. For HTTP/2 and HTTP/3 that NetVConnection is shared by every stream
on the connection, so a single aborted stream stopped the session from
reading frames for all of the others. Route the shutdown through the
transaction instead: Http2Stream and HQTransaction already implement
do_io_shutdown() as a deliberate no-op for exactly this reason, and
HTTP/1.x is unaffected because ProxyTransaction forwards to the session's
NetVConnection.
This is the remaining half of #12529. That change was written to address
two regressions from #12502, the second being "HTTP/2 connection is
closed if a stream is reset even if other streams are alive", with the
stated approach of calling ProxyTransaction::do_io_shutdown() instead of
NetVConnection::do_io_shutdown(). It converted the two branches it added
but left the pre-existing IO_SHUTDOWN_READ branch calling the
NetVConnection directly, so the connection-wide shutdown survived for the
case where the tunnel still has a consumer besides the client. A response
transform reaches that case readily: the transform stage runs the whole
body before anything is written back, so the stream has no write of its
own and a client reset arrives as an EOS on the stream's read VIO.
Clearing the session's read VIO buffer is not inert, because
Http2CommonSession still holds that VIO and re-enables it every 128
frames through HTTP2_SESSION_EVENT_REENABLE. A release build then takes
the ntodo() <= 0 path in net_read_io and the connection stalls silently,
failing every in-flight stream; a debug build aborts on
SSLNetVConnection's
ink_assert(buf.writer()), the assertion reportedin #9448.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com