Skip to content

fix(http2): do not reserve capacity before body data is available - #4061

Merged
seanmonstar merged 2 commits into
masterfrom
sean/omooyxqukpvl
May 4, 2026
Merged

fix(http2): do not reserve capacity before body data is available#4061
seanmonstar merged 2 commits into
masterfrom
sean/omooyxqukpvl

Conversation

@seanmonstar

@seanmonstar seanmonstar commented May 4, 2026

Copy link
Copy Markdown
Member

Continuing #4051

Closes #4003
Closes #4051

barry3406 and others added 2 commits May 4, 2026 16:40
PipeToSendStream used to call `reserve_capacity(1)` at the top of every
poll iteration as a probe, before asking the body for the next chunk.
The reservation is immediately assigned from the connection-level
flow-control window, and while the stream is parked waiting for more
body data the reservation keeps the last byte of the connection window
pinned to the stream.

Against peers that only emit a WINDOW_UPDATE once their receive window
is fully exhausted (for example Bun's built-in HTTP/2 server, as well
as other implementations with a similar strategy), this one-byte
reservation is enough to deadlock a second concurrent stream: the
connection window never drops to zero on the peer, so no WINDOW_UPDATE
is ever sent, and the second stream can never get any capacity.

Restructure the loop so it polls the body for the next frame first and
only reserves capacity equal to the chunk's exact size. The polled
chunk is stashed in a new `buffered_data` field so it survives the
`poll_capacity` wait across `Poll::Pending` returns without being
dropped. Zero-length data frames are forwarded immediately without
touching the reservation. `poll_reset` is now registered at the top of
every iteration so RST_STREAM still wakes the task while it waits for
either more body data or more capacity.

Add a regression test that pairs a streaming request filling the
connection window with a second one-byte request, talking to a raw
h2::server that never releases recv capacity, and asserts the second
request reaches the server.

Closes #4003
@seanmonstar
seanmonstar force-pushed the sean/omooyxqukpvl branch from d6195d8 to e980da5 Compare May 4, 2026 20:41
@seanmonstar
seanmonstar merged commit 99f2434 into master May 4, 2026
22 checks passed
@seanmonstar
seanmonstar deleted the sean/omooyxqukpvl branch May 4, 2026 20:49
sandersaares added a commit to sandersaares/hyper that referenced this pull request Aug 4, 2026
…eration

`PipeToSendStream::poll` calls `SendStream::poll_reset` as the first
statement of its send loop. Every one of those calls clones the current
waker into h2's one-shot `send_task` slot and drops the waker stored
there before, and every call takes h2's connection-wide mutex.

The loop runs more often than it used to: since hyperium#4061 the body frame is
polled first and stashed in `buffered_data`, so a single chunk drives an
extra iteration. Measured against a server that grants send capacity on
the third poll, one 10240-byte body chunk now costs four `poll_reset`
calls where 1.9.0 cost one, taking waker registrations per request from
three to six. Runtimes with a more expensive waker than tokio's pay that
directly; a load client on such a runtime regressed about 20% in CPU time
per request across the 1.9.0 -> 1.10.1 bump, with no change in syscall or
allocation counts.

Most of those registrations were redundant. h2 registers on the same
one-shot slot from `poll_capacity`, and reports a reset there as
`Ready(None)`, so a pipe parked on send capacity is already reachable by
a RST_STREAM. Waiting on the body is the only suspension point h2 knows
nothing about.

So the reset check is now hoisted out of the loop and uses the
non-registering `SendStream::reset_reason`, while `poll_reset` is called
only when `poll_frame` actually returns `Pending`. Because `ready!` on
`poll_capacity` returns from `poll` rather than from the inner loop, a
reset observed while parked on capacity still re-enters at the hoisted
check and still surfaces the peer's `Reason`, not the generic
"send stream capacity unexpectedly closed" message.

This takes the same workload to three reset checks and two waker
registrations per request.
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.

H2 streams reserve 1 more byte than needed from the connection, leading to deadlock

2 participants