Skip to content

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

Merged
seanmonstar merged 1 commit into
hyperium: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 into
hyperium:masterfrom
ArniDagur:fix/no-panic-on-cancel-after-reserve

Conversation

@ArniDagur

@ArniDagur ArniDagur commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

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
bmwill added a commit to MystenLabs/sui-rust-sdk that referenced this pull request Jul 15, 2026
h2 versions before 0.4.14 have several stream-cancellation flow-control
accounting bugs (hyperium/h2#893, #896, #897, #898, and #913, fixed
across 0.4.14 and 0.4.15) that can permanently wedge a multiplexed gRPC
connection: leaked connection send-window capacity, RST_STREAM frames
that are never transmitted, and permanently missed `poll_capacity`
wakeups. Add an explicit hyper 1.10 floor to sui-rpc (hyper 1.10 raised
its h2 floor to 0.4.14 for the same reason) so downstream consumers
cannot resolve a broken h2. The lockfile is not tracked in this repo, so
the manifest floor is the durable guard.
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