Repository navigation
Conversation
Problem: Lowering SETTINGS_INITIAL_WINDOW_SIZE shrinks each stream's recv window when the peer ACKs our SETTINGS. That can make capacity the application already released (held back for batching) eligible for a WINDOW_UPDATE, but apply_local_settings never queued the stream. The peer could be left with a non-positive window and no further DATA to trigger an update, so the stream stalled. Solution: After dec_recv_window, queue the stream in pending_window_updates when unclaimed_capacity() is Some and wake the connection task, mirroring release_capacity. Validation: New h2-tests case client_decrease_initial_window_size_sends_released_capacity times out waiting for the WINDOW_UPDATE on master and passes with the fix. cargo test -p h2-tests, cargo test -p h2 --lib, fmt and clippy pass. Fixes hyperium#976
This branch has not been deployed
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.
Fixes #976
When the local
SETTINGS_INITIAL_WINDOW_SIZEis lowered, each stream's receive window is shrunk once the peer ACKs the SETTINGS. That can make capacity the application has already released, but which was being held back to batch WINDOW_UPDATEs, due to be sent.apply_local_settingsnever queued the stream, so if the peer had no more DATA to send (its window was now non-positive), nothing triggered the update and the stream stalled.Changes:
Recv::apply_local_settingsnow takes the connection task waker.dec_recv_window, ifunclaimed_capacity()isSome, the stream is pushed ontopending_window_updatesand the task is woken, the same wayrelease_capacitydoes it.Streams::apply_local_settingspasses&mut me.actions.taskthrough.Test:
client_decrease_initial_window_size_sends_released_capacityintests/h2-tests/tests/flow_control.rs. The client receives and releases 16384 bytes (below the update threshold), then lowers the initial window to 1024. After the SETTINGS ACK it expectsWINDOW_UPDATE(1, 16384). The test times out waiting for that frame without the fix and passes with it.The new test passed 60 single runs and 10 full
flow_controlruns in a row locally. The window update is queued while the SETTINGS ACK is handled inpoll2and flushed bypoll_completein the sameConnection::poll, so the ordering does not depend on timing.