Skip to content

fix: Avoid capacity leak or panic when a stream is cancelled after reserve_capacity#893

Merged
seanmonstar merged 1 commit intohyperium:masterfrom
ArniDagur:fix/no-panic-on-cancel-after-reserve
Apr 28, 2026
Merged

fix: Avoid capacity leak or panic when a stream is cancelled after reserve_capacity#893
seanmonstar merged 1 commit intohyperium:masterfrom
ArniDagur:fix/no-panic-on-cancel-after-reserve

Conversation

@ArniDagur
Copy link
Copy Markdown
Contributor

@ArniDagur ArniDagur commented Apr 28, 2026

The following triggers a bug

let (resp, mut send) = client.send_request(req, false)?;
send.reserve_capacity(N);
send.send_reset(Reason::CANCEL);

In builds with debug assertions enabled, we panic:

thread 'reserve_capacity_then_cancel_does_not_leak' panicked at src/proto/streams/prioritize.rs:445:9:
state=Closed(Error(Reset(StreamId(1), CANCEL, User)))
stack backtrace:
   0: __rustc::rust_begin_unwind
   1: core::panicking::panic_fmt
   2: h2::proto::streams::prioritize::Prioritize::try_assign_capacity
             at src/proto/streams/prioritize.rs:445:9
   3: h2::proto::streams::prioritize::Prioritize::poll_complete
             at src/proto/streams/prioritize.rs:535:22
   4: h2::proto::streams::send::Send::poll_complete
             at src/proto/streams/send.rs:350:14
   5: h2::proto::streams::streams::Inner::poll_complete
             at src/proto/streams/streams.rs:906:34
   6: h2::proto::streams::streams::Streams<B,P>::poll_complete
             at src/proto/streams/streams.rs:186:12
   7: h2::proto::connection::Connection<T,P,B>::poll
             at src/proto/connection.rs:287:55
   8: <h2::client::Connection<T,B> as core::future::future::Future>::poll
             at src/client.rs:1441:33

In production builds, it instead leaks capacity, since the assert is compiled out.

@ArniDagur ArniDagur force-pushed the fix/no-panic-on-cancel-after-reserve branch 2 times, most recently from 47f487a to 03d67a4 Compare April 28, 2026 01:49
@ArniDagur ArniDagur marked this pull request as draft April 28, 2026 02:07
@ArniDagur ArniDagur force-pushed the fix/no-panic-on-cancel-after-reserve branch from 03d67a4 to e9efaba Compare April 28, 2026 02:39
@ArniDagur ArniDagur marked this pull request as ready for review April 28, 2026 02:44
);
// The stream may have been reset or closed since capacity was requested.
if !stream.state.is_send_streaming() && stream.buffered_send_data == 0 {
return;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think, worth a trace, or is it not interesting enough?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion here, but I lean towards not adding a trace.

  1. The existing guard for the same condition at another location does not log
  2. I'm not sure what the actionable item would be for someone that sees the log

@seanmonstar seanmonstar merged commit 1e68f99 into hyperium:master Apr 28, 2026
6 checks passed
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