fix: send RST_STREAM immediately for CANCEL resets (#880)#881
Closed
mmishra100 wants to merge 1 commit intohyperium:masterfrom
Closed
fix: send RST_STREAM immediately for CANCEL resets (#880)#881mmishra100 wants to merge 1 commit intohyperium:masterfrom
mmishra100 wants to merge 1 commit intohyperium:masterfrom
Conversation
When an implicit CANCEL reset is scheduled (both ResponseFuture and SendStream dropped), pop_frame() tried to drain buffered DATA through flow control before producing the RST_STREAM. If the send window was exhausted the DATA could never drain, so the RST_STREAM was never sent. Now pop_frame() discards buffered DATA and sends RST_STREAM immediately for CANCEL resets. NO_ERROR resets still drain their data first, since they follow a complete response (RFC 9113 §8.1).
seanmonstar
reviewed
Mar 31, 2026
| // and requires all queued data to be sent first. | ||
| // | ||
| // See https://github.com/hyperium/h2/issues/880. | ||
| if stream.state.get_scheduled_reset() == Some(Reason::CANCEL) { |
Member
There was a problem hiding this comment.
I appreciate the comments just above pointing out that NO_ERROR likely should keep sending. But, I'm wondering, is CANCEL the only reason to drop all the data? Shouldn't any other reset tear it all down?
ArniDagur
added a commit
to ArniDagur/h2
that referenced
this pull request
Apr 30, 2026
A revival of hyperium#881 Implicit resets rely on `pop_frame` reaching the `None` arm to emit `RST_STREAM`. If `DATA` is queued but the stream has zero window capacity to send it, `pop_frame` hits this check ``` if sz > 0 && stream_capacity == 0 { tracing::trace!("stream capacity is 0"); // The stream has no more capacity, this can // happen if the remote reduced the stream // window. In this case, we need to buffer the // frame and wait for a window update... stream.pending_send.push_front(buffer, frame.into()); continue; } ``` The frame is pushed back, but the stream is not re-enqueued until we receive a window update for it. If that update is never received, the stream reset will be delayed indefinitely (we're at the mercy of the remote). In debug builds this crashes with ``` panicked at src/proto/streams/counts.rs:282:13: assertion failed: !self.has_streams() ``` when the connection is dropped. Implicit resets can occur under three circumstances: * `CANCEL`: In `maybe_cancel`, when all user handles are dropped for a stream that is still open * `NO_ERROR`: In `maybe_cancel`, when all user handles are dropped for a server stream where the response is already queued * `PROTOCOL_ERROR`: From the oversized headers path in `streams.rs`, after a rejection response is sent For context, `NO_ERROR` for a stream reset is only supposed to be sent after a complete response. Buffering data + response, and then dropping the stream handles sounds like a relatively normal thing to do, and I think we want to support it. However, for the other cases we should just fast-track stream reset and delete the buffered data, as we don't care about the stream anymore. Sending it would be wasteful.
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
2 participants
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.
When an implicit CANCEL reset is scheduled (both ResponseFuture and SendStream dropped), pop_frame() tried to drain buffered DATA through flow control before producing the RST_STREAM. If the send window was exhausted the DATA could never drain, so the RST_STREAM was never sent.
Now pop_frame() discards buffered DATA and sends RST_STREAM immediately for CANCEL resets. NO_ERROR resets still drain their data first, since they follow a complete response (RFC 9113 §8.1).