Skip to content

Commit 0d7e1a7

Browse files
committed
fix: poll_capacity must not return Ready(Some(Ok(0)))
A WINDOW_UPDATE and SETTINGS decrease in the same poll can cancel each other out, resulting in zero capacity. Return `Pending`, rather than `Ready(Ok(0))`. See #270
1 parent 30998f2 commit 0d7e1a7

3 files changed

Lines changed: 66 additions & 2 deletions

File tree

src/proto/streams/send.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,16 @@ impl Send {
376376

377377
stream.send_capacity_inc = false;
378378

379-
Poll::Ready(Some(Ok(self.capacity(stream))))
379+
let capacity = self.capacity(stream);
380+
381+
// If capacity has been reduced to zero, for example due to a race
382+
// with a SETTINGS frame, return Pending instead of Ready(Ok(0)).
383+
if capacity == 0 {
384+
stream.wait_send(cx);
385+
return Poll::Pending;
386+
}
387+
388+
Poll::Ready(Some(Ok(capacity)))
380389
}
381390

382391
/// Current available stream send capacity

src/share.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ impl<B: Buf> SendStream<B> {
308308
/// increased by the connection. Note that `n` here represents the **total**
309309
/// amount of assigned capacity at that point in time. It is also possible
310310
/// that `n` is lower than the previous call if, since then, the caller has
311-
/// sent data.
311+
/// sent data. `n` will always be greater than zero.
312312
pub fn poll_capacity(&mut self, cx: &mut Context) -> Poll<Option<Result<usize, crate::Error>>> {
313313
self.inner
314314
.poll_capacity(cx)

tests/h2-tests/tests/flow_control.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2670,3 +2670,58 @@ async fn poll_capacity_woken_on_library_reset() {
26702670
join(srv, client).await;
26712671
}
26722672
}
2673+
2674+
/// A WINDOW_UPDATE followed by a SETTINGS decrease can cancel each other out, resulting
2675+
/// in zero capacity. `poll_capacity` must return `Pending` (not `Ready(Ok(0))`) in that case.
2676+
#[tokio::test]
2677+
async fn poll_capacity_window_update_settings_race() {
2678+
h2_support::trace_init!();
2679+
let (io, mut srv) = mock::new();
2680+
2681+
let mut settings = frame::Settings::default();
2682+
settings.set_initial_window_size(Some(0));
2683+
2684+
let srv = async move {
2685+
let settings = srv.assert_client_handshake_with_settings(settings).await;
2686+
assert_default_settings!(settings);
2687+
2688+
srv.recv_frame(frames::headers(1).request("POST", "https://example.com/"))
2689+
.await;
2690+
idle_ms(50).await;
2691+
2692+
// Give stream capacity then immediately take it back
2693+
srv.send_frame(frames::window_update(1, 1024)).await;
2694+
srv.send_frame(frames::settings().initial_window_size(0))
2695+
.await;
2696+
srv.recv_frame(frames::settings_ack()).await;
2697+
2698+
// Now give real, usable capacity
2699+
srv.send_frame(frames::window_update(0, 11)).await;
2700+
srv.send_frame(frames::window_update(1, 11)).await;
2701+
srv.recv_frame(frames::data(1, "hello world").eos()).await;
2702+
srv.send_frame(frames::headers(1).response(200).eos()).await;
2703+
};
2704+
2705+
let h2 = async move {
2706+
let (mut client, mut h2) = client::handshake(io).await.unwrap();
2707+
let request = Request::builder()
2708+
.method(Method::POST)
2709+
.uri("https://example.com/")
2710+
.body(())
2711+
.unwrap();
2712+
2713+
let (response, mut stream) = client.send_request(request, false).unwrap();
2714+
stream.reserve_capacity(11);
2715+
2716+
// `wait_for_capacity` panics if `poll_capacity` ever returns `Ok(0)`
2717+
let mut stream = h2.drive(util::wait_for_capacity(stream, 11)).await;
2718+
stream.send_data("hello world".into(), true).unwrap();
2719+
2720+
let response = h2.drive(response).await.unwrap();
2721+
assert_eq!(response.status(), StatusCode::OK);
2722+
2723+
h2.await.unwrap();
2724+
};
2725+
2726+
join(srv, h2).await;
2727+
}

0 commit comments

Comments
 (0)