fix(http2): do not reserve capacity before body data is available#4051
Open
barry3406 wants to merge 1 commit intohyperium:masterfrom
Open
fix(http2): do not reserve capacity before body data is available#4051barry3406 wants to merge 1 commit intohyperium:masterfrom
barry3406 wants to merge 1 commit intohyperium:masterfrom
Conversation
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
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
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.
PipeToSendStreamused to callreserve_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_UPDATEonce 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 noWINDOW_UPDATEis 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_datafield onPipeToSendStreamso it survives thepoll_capacitywait acrossPoll::Pendingreturns without being dropped. Zero-length data frames are forwarded immediately without touching the reservation.poll_resetis now registered at the top of every iteration, soRST_STREAMstill 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::serverthat never releases recv capacity, and asserts that the second request reaches the server. The test deadlocks onmasterand passes with this change.Closes #4003