Skip to content

Send RST_STREAM for a cancelled half-closed stream - #710

Merged
ok2c merged 1 commit into
apache:masterfrom
rp-arielrodriguez:h2-cancel-rst
Sep 30, 2026
Merged

ok2c merged 1 commit into
apache:masterfrom
rp-arielrodriguez:h2-cancel-rst

Conversation

@rp-arielrodriguez

@rp-arielrodriguez rp-arielrodriguez commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

We found this with HTTP/2 multiplexing in HttpClient 5: when a GET timed out
on a shared connection and the server stayed silent, no RST_STREAM was sent.
The server kept working on the request, and the stream kept its slot.

H2Stream#abort() sets cancelled and calls requestOutput(), but the flag is
checked only in consumeHeader, consumeData and consumePromise. The output
loop skips local-closed streams, so a half-closed stream with a silent peer is
never reset.

Fix: H2Stream#resetIfCancelled() sends RST_STREAM(CANCEL) for a cancelled
half-closed stream. onOutput() calls it for each stream, outside the
connection window check, because RST_STREAM is not subject to flow control.

Backport: the main code applies cleanly to 5.4.x; the test file needs a small
merge.

Tests:

  • New: testAbortAfterLocalEndStreamSendsRstStreamWithoutInboundFrames (fails
    on master, passes with the fix).
  • New: testAbortAfterLocalEndStreamSendsRstStreamWithNoConnectionWindow (fails
    on master and on the previous revision of this PR, passes with the fix).
  • ./mvnw -pl httpcore5-testing -am test: httpcore5 1960, httpcore5-h2 388,
    httpcore5-testing 443 (4 skipped), all pass. Checkstyle and RAT pass.

I used an AI assistant (Claude Code) for this change. I reviewed all of it and
I can answer questions about it.

@arturobernalg

Copy link
Copy Markdown
Member

@rp-arielrodriguez RST_STREAM is not subject to flow control. Shouldn't a pending stream reset be processed regardless of connOutputWindow?

@rp-arielrodriguez

rp-arielrodriguez commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor Author

@arturobernalg Agreed, thanks. I will apply the window check only to normal stream output, so a pending reset is sent even when connOutputWindow is 0, and add a test for it. Should the reset also skip the remote SETTINGS ACK check?

@rp-arielrodriguez
rp-arielrodriguez force-pushed the h2-cancel-rst branch 2 times, most recently from 2561359 to 901d28d Compare September 29, 2026 20:44
A cancelled stream was reset only when a frame arrived from the peer. A
stream that had already sent END_STREAM produces no more output, so a
silent peer never received RST_STREAM(CANCEL).

The multiplexer now resets such a stream on output. RST_STREAM is not
subject to flow control, so the reset does not wait for the connection
output window.

Generated-by: Claude Code (Claude Opus 5.5)
@ok2c
ok2c merged commit 23dc933 into apache:master Sep 30, 2026
12 checks passed
@ok2c

ok2c commented Sep 30, 2026

Copy link
Copy Markdown
Member

@rp-arielrodriguez Cherry-picked to 5.4.x though I had to port the tests manually. Please double-check.

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.

3 participants