Skip to content

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

Open
barry3406 wants to merge 1 commit intohyperium:masterfrom
barry3406:fix/h2-pipe-to-send-stream-reserve-extra-byte
Open

fix(http2): do not reserve capacity before body data is available#4051
barry3406 wants to merge 1 commit intohyperium:masterfrom
barry3406:fix/h2-pipe-to-send-stream-reserve-extra-byte

Conversation

@barry3406
Copy link
Copy Markdown

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 noted in #4003, 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.

The loop is restructured 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 on PipeToSendStream 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.

A regression test 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 that the second request reaches the server. The test deadlocks on master and passes with this change.

Closes #4003

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 hyperium#4003
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

1 participant