Skip to content

netty: reset stream when trailers arrive with END_STREAM unsent - #13079

Open
benjaminp wants to merge 1 commit into
grpc:masterfrom
benjaminp:netty-reset-stream-when-end-stream-blocked
Open

benjaminp wants to merge 1 commit into
grpc:masterfrom
benjaminp:netty-reset-stream-when-end-stream-blocked

Conversation

@benjaminp

Copy link
Copy Markdown
Contributor

transportHeadersReceived() already reset the stream when trailers arrived before the client finished sending, but it used isOutboundClosed() as a stand-in for "END_STREAM has been sent". Those differ whenever END_STREAM is still waiting on flow control: the application has half-closed, yet the frame sits in the remote flow controller. If the server responded and closed the call without reading the rest of the request, it stops returning window, so END_STREAM can never be written and neither side resets the stream.

Decide the reset on the HTTP/2 stream's local state instead, which is only closed once END_STREAM has actually been written.

Fixes #13078.

`transportHeadersReceived()` already reset the stream when trailers arrived before the client finished sending, but it used `isOutboundClosed()` as a stand-in for "END_STREAM has been sent". Those differ whenever END_STREAM is still waiting on flow control: the application has half-closed, yet the frame sits in the remote flow controller. If the server responded and closed the call without reading the rest of the request, it stops returning window, so END_STREAM can never be written and neither side resets the stream.

Decide the reset on the HTTP/2 stream's local state instead, which is only closed once END_STREAM has actually been written.

Fixes grpc#13078.
@benjaminp benjaminp changed the title netty: reset stream when trailers arrive with END_STREAM still queued netty: reset stream when trailers arrive with END_STREAM unsent Sep 25, 2026
@AgraVator AgraVator added the kokoro:force-run Add this label to a PR to tell Kokoro to re-run all tests. Not generally necessary label Sep 26, 2026
@grpc-kokoro grpc-kokoro removed the kokoro:force-run Add this label to a PR to tell Kokoro to re-run all tests. Not generally necessary label Sep 26, 2026
@AgraVator

AgraVator commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Hey, can you please add some test which covers all the branches ?

@benjaminp

Copy link
Copy Markdown
Contributor Author

!isOutboundClosed() is tested by the preexisting inboundTrailersBeforeHalfCloseSendsRstStream

(http2Stream != null && http2Stream.state().localSideOpen()) is tested by the test added in this PR. I suspect http2Stream != null always holds but doing a null-check for defensive programming against a field marked @Nullable.

This branch has not been deployed

No deployments
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.

flow-controlled client streams can be leaked when the server half-closes

3 participants