Skip to content

Conversation

@ghedo
Copy link
Member

@ghedo ghedo commented Oct 14, 2025

h3 streams are meant to be dopped from the internal stream map when both the send and recv side of the stream are finished (this means both sending and received a fin for example).

However currently this only works if the recv side is finished first, and the clean-up happens on the next send_body() or send_response() call. When the opposite happens (e.g. fin is sent before receiving it) the the stream leaks.

This change adds logic to properly check for a stream being completed in that case as well, which involves tracking a "local finished" state for each stream.

When returning a Finished event, the corresponding stream is checked again and if its send-side is also finished then the stream's state is cleared.

Note that this change doesn't cover the case of stream being reset by the local application itself, as the h3 layer doesn't get any signal about that happening right now, so it will need follow-up changes to be fixed.

@ghedo ghedo requested a review from a team as a code owner October 14, 2025 15:57
h3 streams are meant to be dopped from the internal stream map when both
the send and recv side of the stream are finished (this means both
sending and received a `fin` for example).

However currently this only works if the recv side is finished first,
and the clean-up happens on the next `send_body()` or `send_response()`
call. When the opposite happens (e.g. `fin` is sent before receiving it)
the the stream leaks.

This change adds logic to properly check for a stream being completed in
that case as well, which involves tracking a "local finished" state for
each stream.

When returning a `Finished` event, the corresponding stream is checked
again and if its send-side is also finished then the stream's state is
cleared.

Note that this change doesn't cover the case of stream being reset by
the local application itself, as the h3 layer doesn't get any signal
about that happening right now, so it will need follow-up changes to
be fixed.
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.

2 participants