Skip to content

Commit

Permalink
Fix window size decrement of send-closed streams
Browse files Browse the repository at this point in the history
Send-closed streams should be skipped when decreasing window size,
as they are skipped when increasing it.
  • Loading branch information
nox committed Jan 16, 2025
1 parent d7c56f4 commit 513edb3
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 13 deletions.
8 changes: 4 additions & 4 deletions src/proto/streams/flow_control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,16 @@ impl FlowControl {
///
/// This is called after receiving a SETTINGS frame with a lower
/// INITIAL_WINDOW_SIZE value.
pub fn dec_send_window(&mut self, sz: WindowSize) -> Result<(), Reason> {
pub fn dec_send_window(&mut self, sz: WindowSize) {
tracing::trace!(
"dec_window; sz={}; window={}, available={}",
sz,
self.window_size,
self.available
);
// ~~This should not be able to overflow `window_size` from the bottom.~~ wrong. it can.
self.window_size.decrease_by(sz)?;
Ok(())
self.window_size
.decrease_by(sz)
.expect("window_size should be greater than decrement");
}

/// Decrement the recv-side window size.
Expand Down
16 changes: 11 additions & 5 deletions src/proto/streams/send.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,18 +466,24 @@ impl Send {
store.try_for_each(|mut stream| {
let stream = &mut *stream;

if stream.state.is_send_closed() {
tracing::trace!(
"skipping send-closed stream; id={:?}; flow={:?}",
stream.id,
stream.send_flow
);

return Ok(());
}

tracing::trace!(
"decrementing stream window; id={:?}; decr={}; flow={:?}",
stream.id,
dec,
stream.send_flow
);

// TODO: this decrement can underflow based on received frames!
stream
.send_flow
.dec_send_window(dec)
.map_err(proto::Error::library_go_away)?;
stream.send_flow.dec_send_window(dec);

// It's possible that decreasing the window causes
// `window_size` (the stream-specific window) to fall below
Expand Down
6 changes: 2 additions & 4 deletions tests/h2-tests/tests/flow_control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1859,7 +1859,7 @@ async fn poll_capacity_wakeup_after_window_update() {
}

#[tokio::test]
async fn window_size_decremented_past_zero() {
async fn window_size_does_not_underflow() {
h2_support::trace_init!();
let (io, mut client) = mock::new();

Expand Down Expand Up @@ -1891,9 +1891,7 @@ async fn window_size_decremented_past_zero() {
let builder = server::Builder::new();
let mut srv = builder.handshake::<_, Bytes>(io).await.expect("handshake");

// just keep it open
let res = poll_fn(move |cx| srv.poll_closed(cx)).await;
tracing::debug!("{:?}", res);
poll_fn(move |cx| srv.poll_closed(cx)).await.unwrap();
};

join(client, srv).await;
Expand Down

0 comments on commit 513edb3

Please sign in to comment.