From f01cc5417a95779a46f95504ab1bda47eb47b564 Mon Sep 17 00:00:00 2001 From: Haga Rak Date: Fri, 20 Mar 2026 11:32:49 +0100 Subject: [PATCH] Fix H2 window flow control bugs causing flaky CI timeouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use negotiated SETTINGS_INITIAL_WINDOW_SIZE for new streams instead of hardcoded 65535 (OverallWindowSize). Streams created after the SETTINGS exchange were missing the server's initial window, causing the client to block on an exhausted stream window while the server believed it had plenty — root cause of 30s timeouts on large request bodies. - Set Remote PeerSetting.WindowSize default to 65535 (RFC 9113 §6.5.2). - Fix _initDone = false (should be true) in H2ConnectionPool.Init(), which allowed double-initialization of the connection. - Fix lost-wakeup race in WindowSizeHolder: wake a waiter whenever the window is positive, not only on the ≤0 → >0 transition. Concurrent UpdateWindowSize calls could silently skip the wake. - Make StreamPool.ShouldWindowUpdate thread-safe with Interlocked ops. - Add larger payload (150002 * 16) to Http2ClientHandler test coverage. --- src/Fluxzy.Core/Clients/H2/H2ConnectionPool.cs | 2 +- src/Fluxzy.Core/Clients/H2/H2StreamSetting.cs | 4 +++- src/Fluxzy.Core/Clients/H2/StreamPool.cs | 15 ++++++++------- src/Fluxzy.Core/Clients/H2/StreamWorker.cs | 2 +- src/Fluxzy.Core/Clients/H2/WindowSizeHolder.cs | 8 ++++++-- .../UnitTests/Handlers/Http2ClientHandler.cs | 2 +- 6 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/Fluxzy.Core/Clients/H2/H2ConnectionPool.cs b/src/Fluxzy.Core/Clients/H2/H2ConnectionPool.cs index c5c9cb2e..861ca663 100644 --- a/src/Fluxzy.Core/Clients/H2/H2ConnectionPool.cs +++ b/src/Fluxzy.Core/Clients/H2/H2ConnectionPool.cs @@ -107,7 +107,7 @@ public void Init() if (_initDone) return; - _initDone = false; + _initDone = true; //_baseStream.Write(Preface); SettingHelper.WriteWelcomeSettings(H2Constants.Preface, _baseStream, Setting, _logger); diff --git a/src/Fluxzy.Core/Clients/H2/H2StreamSetting.cs b/src/Fluxzy.Core/Clients/H2/H2StreamSetting.cs index a6bd83f4..e169a92b 100644 --- a/src/Fluxzy.Core/Clients/H2/H2StreamSetting.cs +++ b/src/Fluxzy.Core/Clients/H2/H2StreamSetting.cs @@ -13,7 +13,9 @@ public class H2StreamSetting SettingsMaxConcurrentStreams = 256 }; - public PeerSetting Remote { get; set; } = new(); + public PeerSetting Remote { get; set; } = new() { + WindowSize = 65535 // HTTP/2 spec default (RFC 9113 §6.5.2) + }; public int SettingsHeaderTableSize { get; set; } = 65536; diff --git a/src/Fluxzy.Core/Clients/H2/StreamPool.cs b/src/Fluxzy.Core/Clients/H2/StreamPool.cs index 91abe94c..79aef0b2 100644 --- a/src/Fluxzy.Core/Clients/H2/StreamPool.cs +++ b/src/Fluxzy.Core/Clients/H2/StreamPool.cs @@ -121,17 +121,18 @@ public void OnGoAway(Exception? ex) public int ShouldWindowUpdate(int dataLength) { - var windowIncrement = 0; + var newValue = Interlocked.Add(ref _overallWindowSize, dataLength); + var threshold = (int)(0.5 * Context.Setting.Local.WindowSize); - _overallWindowSize += dataLength; + if (newValue > threshold) { + // CAS to claim the accumulated value + var claimed = Interlocked.Exchange(ref _overallWindowSize, 0); - if (_overallWindowSize > 0.5 * Context.Setting.Local.WindowSize) { - windowIncrement = _overallWindowSize; - - _overallWindowSize = 0; + if (claimed > 0) + return claimed; } - return windowIncrement; + return 0; } } } diff --git a/src/Fluxzy.Core/Clients/H2/StreamWorker.cs b/src/Fluxzy.Core/Clients/H2/StreamWorker.cs index 22003515..5663615a 100644 --- a/src/Fluxzy.Core/Clients/H2/StreamWorker.cs +++ b/src/Fluxzy.Core/Clients/H2/StreamWorker.cs @@ -48,7 +48,7 @@ public StreamWorker( _resetTokenSource = resetTokenSource; RemoteWindowSize = new WindowSizeHolder(parent.Context.Logger, - parent.Context.Setting.OverallWindowSize, + parent.Context.Setting.Remote.WindowSize, streamIdentifier); _logger = parent.Context.Logger; diff --git a/src/Fluxzy.Core/Clients/H2/WindowSizeHolder.cs b/src/Fluxzy.Core/Clients/H2/WindowSizeHolder.cs index 81914ff7..073a0599 100644 --- a/src/Fluxzy.Core/Clients/H2/WindowSizeHolder.cs +++ b/src/Fluxzy.Core/Clients/H2/WindowSizeHolder.cs @@ -81,8 +81,12 @@ public void UpdateWindowSize(int windowSizeIncrement) break; } - // Signal ONE waiter if we crossed from ≤0 to >0. - if (before <= 0 && after > 0) + // Signal ONE waiter whenever window is positive — not just on the + // 0→positive transition. When two concurrent UpdateWindowSize calls + // race, only one observes before≤0; the other sees before>0 and skips + // the wake even though a waiter may still be queued. WakeOneWaiter is + // cheap (lock + empty-check) when no waiters exist. + if (after > 0) { WakeOneWaiter(); } diff --git a/test/Fluxzy.Tests/UnitTests/Handlers/Http2ClientHandler.cs b/test/Fluxzy.Tests/UnitTests/Handlers/Http2ClientHandler.cs index 49ac2b00..5e13c3ad 100644 --- a/test/Fluxzy.Tests/UnitTests/Handlers/Http2ClientHandler.cs +++ b/test/Fluxzy.Tests/UnitTests/Handlers/Http2ClientHandler.cs @@ -19,7 +19,7 @@ public static IEnumerable GetHttpMethods { get { - int[] checkLength = { 0, 152, 12464, 150002 }; + int[] checkLength = { 0, 152, 12464, 150002, 150002 * 16 }; foreach (var length in checkLength) {