diff --git a/src/GaudiHTTP.Tests/Pooling/ConnectionObjectPoolSpec.cs b/src/GaudiHTTP.Tests/Pooling/ConnectionObjectPoolSpec.cs index b8a8a28d..aaeb444a 100644 --- a/src/GaudiHTTP.Tests/Pooling/ConnectionObjectPoolSpec.cs +++ b/src/GaudiHTTP.Tests/Pooling/ConnectionObjectPoolSpec.cs @@ -61,29 +61,29 @@ public void Dispose_on_a_Poolable_returns_it_to_the_singleton_pool_reset() ctx.Return(b); } - private sealed class NeverRentedCounter : Poolable + private sealed class ReturnFirstCounter : Poolable { public int Value; - public int ResetCount; - protected override void OnReset() - { - Value = 0; - ResetCount++; - } + protected override void OnReset() => Value = 0; } [Fact(Timeout = 5000)] - public void Return_without_prior_rent_resets_and_discards_instead_of_throwing() + public void Return_before_any_rent_must_not_throw_and_later_rent_reuses_the_instance() { - // Directly-constructed poolables (tests injecting fakes, abort paths racing the first - // Rent of the process) dispose without a pool existing for their type. That must - // behave like returning to a full pool — reset and drop — not throw. - var a = new NeverRentedCounter { Value = 42 }; + // A pooled object constructed directly (new, not rented) disposes itself back into the + // pool. That Return may be the very first pool touch for the type — it must not require + // a factory registration. + var ctx = ConnectionObjectPool.Instance; + var a = new ReturnFirstCounter { Value = 5 }; a.Dispose(); - Assert.Equal(1, a.ResetCount); - Assert.Equal(0, a.Value); + var b = ctx.Rent(static () => new ReturnFirstCounter()); + + Assert.Same(a, b); + Assert.Equal(0, b.Value); + + ctx.Return(b); } [Fact(Timeout = 5000)] diff --git a/src/GaudiHTTP.Tests/Protocol/Body/FlowControlledBodyPumpSpec.cs b/src/GaudiHTTP.Tests/Protocol/Body/FlowControlledBodyPumpSpec.cs index 57ea0a31..c0ee5790 100644 --- a/src/GaudiHTTP.Tests/Protocol/Body/FlowControlledBodyPumpSpec.cs +++ b/src/GaudiHTTP.Tests/Protocol/Body/FlowControlledBodyPumpSpec.cs @@ -1,3 +1,5 @@ +using System.Collections; +using System.Reflection; using Akka.Actor; using GaudiHTTP.Protocol.Body; using GaudiHTTP.Protocol.Syntax.Http2; @@ -89,9 +91,6 @@ private static void DrainToCompletion(FlowControlledBodyPump scheduler, FakeTarg case BodyReadComplete rc: scheduler.HandleReadComplete(rc.StreamId, rc.BytesRead); break; - case BodyReadContinue dc: - scheduler.HandleBodyReadContinue(dc.StreamId); - break; } } } @@ -369,6 +368,77 @@ public void OrphanRefund_should_return_reserved_window_when_stream_cancelled_dur Assert.Equal(windowBefore, flow.ConnectionSendWindow); } + [Fact(Timeout = 5000)] + public void FailedRead_should_refund_reserved_window() + { + var target = new FakeTarget(); + var flow = MakeFlow(); + var scheduler = new FlowControlledBodyPump(target, flow, new CancellationTokenSource(), chunkSize: 1 * 1024, hardCap: 16); + + flow.InitStreamSendWindow(1); + var windowBefore = flow.ConnectionSendWindow; + + // Async read path so the reservation is held while the read is in flight. + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var blockingStream = new DelegatingReadStream((_, _) => new ValueTask(tcs.Task)); + scheduler.Register(1, blockingStream, null, CancellationToken.None); + + Assert.Equal(windowBefore - 1 * 1024, flow.ConnectionSendWindow); + + // Read fails while the stream is still active (not orphaned): nothing was + // sent, so the full reservation must be refunded. + scheduler.HandleReadFailed(1, new IOException("read failed")); + + Assert.Single(target.Failed); + Assert.Equal(windowBefore, flow.ConnectionSendWindow); + } + + [Fact(Timeout = 5000)] + public void Cancel_should_dispose_queued_non_inflight_slot_immediately_even_when_window_stays_exhausted() + { + var target = new FakeTarget(); + var flow = MakeFlow(connWindow: 65535); + // hardCap = 1: only one async read may be in flight at a time. + var scheduler = new FlowControlledBodyPump(target, flow, new CancellationTokenSource(), chunkSize: 64, hardCap: 1); + + flow.InitStreamSendWindow(1); + flow.InitStreamSendWindow(3); + + // Stream 1 occupies the only read slot with a read that never completes. + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var blockingStream = new DelegatingReadStream((_, _) => new ValueTask(tcs.Task)); + scheduler.Register(1, blockingStream, null, CancellationToken.None); + + // Stream 3 has an open window, so it is enqueued — but _asyncInFlight (1) already meets + // hardCap (1), so it never starts a read: it sits in the ready queue, not in-flight and + // not window-blocked. + scheduler.Register(3, MakeBody(100), 100, CancellationToken.None); + + var activeSlots = GetActiveSlots(scheduler); + Assert.True(activeSlots.Contains(3)); + + // Cancel stream 3 while queued. The connection window never opens again afterward, + // simulating a stalled connection — a deferred-dispose discipline would leave the + // cancelled slot (and its pooled buffer) alive until Cleanup(); the correct discipline + // disposes it immediately, matching MultiplexedBodyPump. + scheduler.Cancel(3); + + Assert.False(activeSlots.Contains(3)); + + // Opening the window afterward must not resurrect or emit anything for stream 3. + flow.OnSendWindowUpdate(0, 65535); + scheduler.OnWindowUpdate(0); + + Assert.DoesNotContain(3, target.Completed); + Assert.DoesNotContain(target.Emitted, e => e.StreamId == 3); + } + + private static IDictionary GetActiveSlots(FlowControlledBodyPump scheduler) + { + var field = typeof(FlowControlledBodyPump).GetField("_activeSlots", BindingFlags.NonPublic | BindingFlags.Instance)!; + return (IDictionary)field.GetValue(scheduler)!; + } + [Fact(Timeout = 5000)] public void WindowBlocked_should_block_when_available_below_half_chunk() { diff --git a/src/GaudiHTTP.Tests/Protocol/Body/MultiplexedBodyPumpSpec.cs b/src/GaudiHTTP.Tests/Protocol/Body/MultiplexedBodyPumpSpec.cs index 7786af7d..c6518271 100644 --- a/src/GaudiHTTP.Tests/Protocol/Body/MultiplexedBodyPumpSpec.cs +++ b/src/GaudiHTTP.Tests/Protocol/Body/MultiplexedBodyPumpSpec.cs @@ -64,9 +64,6 @@ private static void DrainToCompletion( case BodyReadComplete rc: pump.HandleReadComplete(rc.StreamId, rc.BytesRead); break; - case BodyReadContinue dc: - pump.HandleBodyReadContinue(dc.StreamId); - break; } } } @@ -91,9 +88,6 @@ private static void DriveReadsWithoutCapacity( case BodyReadComplete rc: pump.HandleReadComplete(rc.StreamId, rc.BytesRead); break; - case BodyReadContinue dc: - pump.HandleBodyReadContinue(dc.StreamId); - break; } } } @@ -199,19 +193,18 @@ public void SyncFastPath_should_drain_small_body_inline() } [Fact(Timeout = 5000)] - public void Sync_reads_should_drain_body_larger_than_starvation_threshold() + public void Many_consecutive_sync_reads_should_drain_fully() { var target = new FakeTarget(); - // Use small chunk so many sync reads happen + // Small chunk so the body needs dozens of consecutive sync reads to drain. var pump = MakePump(target, chunkSize: 16); pump.Register(1L, MakeBody(65 * 16), contentLength: null, CancellationToken.None); - // Starvation guard fires at 64 consecutive reads and dispatches BodyReadContinue. - // DrainToCompletion processes those messages to drive the pump to completion. DrainToCompletion(pump, target); var total = target.Emitted.Where(e => !e.EndStream).Sum(e => e.Data.Length); - Assert.True(total > 0, "Expected at least some data emitted before starvation guard fired"); + Assert.Equal(65 * 16, total); + Assert.Single(target.Completed); } [Fact(Timeout = 5000)] diff --git a/src/GaudiHTTP.Tests/Protocol/Body/SerialBodyPumpSpec.cs b/src/GaudiHTTP.Tests/Protocol/Body/SerialBodyPumpSpec.cs index 270974ae..8cc890f9 100644 --- a/src/GaudiHTTP.Tests/Protocol/Body/SerialBodyPumpSpec.cs +++ b/src/GaudiHTTP.Tests/Protocol/Body/SerialBodyPumpSpec.cs @@ -60,9 +60,6 @@ private static void DrainToCompletion( case BodyReadComplete rc: pump.HandleReadComplete(rc.BytesRead); break; - case BodyReadContinue: - pump.HandleBodyReadContinue(); - break; } } } @@ -70,7 +67,7 @@ private static void DrainToCompletion( /// /// Target that calls OnCapacityAvailable() synchronously after EmitDataFrames, /// simulating a consumer that immediately signals capacity after each chunk. - /// Uses a direct-dispatch actor ref so pump messages (BodyReadComplete, BodyReadContinue) + /// Uses a direct-dispatch actor ref so pump messages (BodyReadComplete) /// are processed inline as they arrive — no external drain loop needed. /// private sealed class AutoResumeTarget : IBodyDrainTarget @@ -119,9 +116,6 @@ protected override void TellInternal(object message, IActorRef sender) case BodyReadComplete rc: owner._pump.HandleReadComplete(rc.BytesRead); break; - case BodyReadContinue: - owner._pump.HandleBodyReadContinue(); - break; } } } @@ -193,9 +187,6 @@ public void OnCapacityAvailable_should_resume_drain_after_capacity_exhaustion() case BodyReadComplete rc: pump.HandleReadComplete(rc.BytesRead); break; - case BodyReadContinue: - pump.HandleBodyReadContinue(); - break; } } @@ -217,9 +208,6 @@ public void OnCapacityAvailable_should_resume_drain_after_capacity_exhaustion() case BodyReadComplete rc: pump.HandleReadComplete(rc.BytesRead); break; - case BodyReadContinue: - pump.HandleBodyReadContinue(); - break; } } } @@ -299,11 +287,10 @@ public void Large_body_should_drain_fully_with_auto_resume() } [Fact(Timeout = 5000)] - public void Sync_reads_should_complete_large_body_with_starvation_guard_via_actor() + public void Many_consecutive_sync_reads_should_drain_fully_with_auto_resume() { var target = new AutoResumeTarget(); - // Small chunk to trigger many sync reads; starvation guard will fire at 64 consecutive reads. - // The AutoResumeTarget processes BodyReadContinue messages inline, so the drain completes fully. + // Small chunk so the body needs dozens of consecutive sync reads to drain. var pump = MakePump(target, chunkSize: 16, maxCapacity: 2); target.SetPump(pump); var totalSize = 65 * 16; @@ -311,7 +298,6 @@ public void Sync_reads_should_complete_large_body_with_starvation_guard_via_acto pump.Register(body, contentLength: null, CancellationToken.None); - // AutoResumeTarget processes BodyReadContinue via DrainMessages() — full body drains. var emittedBytes = target.Emitted.Where(e => !e.EndStream).Sum(e => e.Data.Length); Assert.Equal(totalSize, emittedBytes); Assert.Single(target.Completed); @@ -350,9 +336,6 @@ public void Capacity_grant_during_pending_completion_must_not_corrupt_buffer() case BodyReadComplete rc: pump.HandleReadComplete(rc.BytesRead); break; - case BodyReadContinue: - pump.HandleBodyReadContinue(); - break; } } diff --git a/src/GaudiHTTP.Tests/Protocol/Semantics/ConnectionRateGuardSpec.cs b/src/GaudiHTTP.Tests/Protocol/Semantics/ConnectionRateGuardSpec.cs new file mode 100644 index 00000000..8490ddbf --- /dev/null +++ b/src/GaudiHTTP.Tests/Protocol/Semantics/ConnectionRateGuardSpec.cs @@ -0,0 +1,142 @@ +using GaudiHTTP.Protocol.Semantics; +using GaudiHTTP.Server; +using GaudiHTTP.Tests.Shared; + +namespace GaudiHTTP.Tests.Protocol.Semantics; + +public sealed class ConnectionRateGuardSpec +{ + private static readonly DataRateOptions Rate = new( + MinRequestBodyDataRate: 100, + MinRequestBodyDataRateGracePeriod: TimeSpan.FromSeconds(2), + MinResponseDataRate: 100, + MinResponseDataRateGracePeriod: TimeSpan.FromSeconds(2)); + + private sealed class FakeClock(long startMs) : TimeProvider + { + private long _nowMs = startMs; + + public void Advance(TimeSpan by) => _nowMs += (long)by.TotalMilliseconds; + + public override DateTimeOffset GetUtcNow() => DateTimeOffset.FromUnixTimeMilliseconds(_nowMs); + } + + [Fact(Timeout = 5000)] + public void ObserveRequest_should_schedule_the_rate_check_timer() + { + var ops = new FakeServerOps(); + var guard = new ConnectionRateGuard(ops, Rate); + + guard.ObserveRequest(0, 10); + + Assert.Contains(ops.ScheduledTimers, t => t.Name == ConnectionRateGuard.TimerName); + } + + [Fact(Timeout = 5000)] + public void ObserveRequest_should_not_reschedule_while_timer_is_active() + { + var ops = new FakeServerOps(); + var guard = new ConnectionRateGuard(ops, Rate); + + guard.ObserveRequest(0, 10); + guard.ObserveResponse(0, 10); + + Assert.Single(ops.ScheduleTimerCalls); + } + + [Fact(Timeout = 5000)] + public void OnTimerFired_should_rearm_when_below_grace_and_still_tracking() + { + var clock = new FakeClock(0); + var ops = new FakeServerOps(); + var guard = new ConnectionRateGuard(ops, Rate, clock); + + guard.ObserveRequest(0, 10); + clock.Advance(TimeSpan.FromSeconds(1)); + + var violated = guard.OnTimerFired(); + + Assert.False(violated); + Assert.Contains(ops.ScheduledTimers, t => t.Name == ConnectionRateGuard.TimerName); + } + + [Fact(Timeout = 5000)] + public void OnTimerFired_should_trip_and_stop_rearming_after_grace_period_elapses() + { + var clock = new FakeClock(0); + var ops = new FakeServerOps(); + var guard = new ConnectionRateGuard(ops, Rate, clock); + + guard.ObserveRequest(0, 10); + + clock.Advance(TimeSpan.FromSeconds(1)); + Assert.False(guard.OnTimerFired()); + + clock.Advance(TimeSpan.FromSeconds(1)); + Assert.False(guard.OnTimerFired()); + + clock.Advance(TimeSpan.FromSeconds(2)); + var violated = guard.OnTimerFired(); + + Assert.True(violated); + } + + [Fact(Timeout = 5000)] + public void OnTimerFired_should_invoke_the_violation_callback_with_entry_counts() + { + var clock = new FakeClock(0); + var ops = new FakeServerOps(); + var guard = new ConnectionRateGuard(ops, Rate, clock); + + guard.ObserveRequest(0, 10); + guard.ObserveResponse(0, 10); + + clock.Advance(TimeSpan.FromSeconds(1)); + guard.OnTimerFired(); + clock.Advance(TimeSpan.FromSeconds(1)); + guard.OnTimerFired(); + clock.Advance(TimeSpan.FromSeconds(2)); + + int? reqCount = null; + int? respCount = null; + guard.OnTimerFired((req, resp) => + { + reqCount = req; + respCount = resp; + }); + + Assert.Equal(1, reqCount); + Assert.Equal(1, respCount); + } + + [Fact(Timeout = 5000)] + public void Remove_should_stop_tracking_the_stream() + { + var clock = new FakeClock(0); + var ops = new FakeServerOps(); + var guard = new ConnectionRateGuard(ops, Rate, clock); + + guard.ObserveRequest(0, 10); + guard.RemoveRequest(0); + + clock.Advance(TimeSpan.FromSeconds(10)); + var violated = guard.OnTimerFired(); + + Assert.False(violated); + } + + [Fact(Timeout = 5000)] + public void Cleanup_should_cancel_the_timer_and_allow_it_to_be_rescheduled() + { + var ops = new FakeServerOps(); + var guard = new ConnectionRateGuard(ops, Rate); + + guard.ObserveRequest(0, 10); + guard.Cleanup(); + + Assert.Contains(ops.CancelledTimers, t => t == ConnectionRateGuard.TimerName); + + guard.ObserveRequest(0, 10); + Assert.Equal(2, ops.ScheduleTimerCalls.Count); + } +} diff --git a/src/GaudiHTTP.Tests/Protocol/Semantics/ReconnectPolicySpec.cs b/src/GaudiHTTP.Tests/Protocol/Semantics/ReconnectPolicySpec.cs new file mode 100644 index 00000000..87a0a1d0 --- /dev/null +++ b/src/GaudiHTTP.Tests/Protocol/Semantics/ReconnectPolicySpec.cs @@ -0,0 +1,80 @@ +using Servus.Akka.Transport; +using GaudiHTTP.Protocol.Semantics; +using GaudiHTTP.Tests.Shared; + +namespace GaudiHTTP.Tests.Protocol.Semantics; + +public sealed class ReconnectPolicySpec +{ + private static TransportOptions MakeTransportOptions() => new TcpTransportOptions + { + Host = "example.com", + Port = 80 + }; + + [Fact(Timeout = 5000)] + public void Start_should_buffer_the_work_and_emit_a_connect_transport() + { + var ops = new FakeClientOps(); + var policy = new ReconnectPolicy(ops, maxAttempts: 3); + + policy.Start("buffered-request", MakeTransportOptions()); + + Assert.Equal(1, policy.Attempts); + Assert.Single(ops.Outbound); + Assert.IsType(ops.Outbound[0]); + } + + [Fact(Timeout = 5000)] + public void TakeBuffered_should_return_the_buffered_work_and_reset_the_attempt_counter() + { + var ops = new FakeClientOps(); + var policy = new ReconnectPolicy(ops, maxAttempts: 3); + + policy.Start("buffered-request", MakeTransportOptions()); + var buffered = policy.TakeBuffered(); + + Assert.Equal("buffered-request", buffered); + Assert.Equal(0, policy.Attempts); + Assert.Null(policy.TakeBuffered()); + } + + [Fact(Timeout = 5000)] + public void OnAttemptFailed_should_retry_and_increment_attempts_below_the_max() + { + var ops = new FakeClientOps(); + var policy = new ReconnectPolicy(ops, maxAttempts: 3); + policy.Start("buffered-request", MakeTransportOptions()); + + var exhausted = policy.OnAttemptFailed(MakeTransportOptions(), out var buffered); + + Assert.False(exhausted); + Assert.Null(buffered); + Assert.Equal(2, policy.Attempts); + Assert.Equal(2, ops.Outbound.Count); + Assert.IsType(ops.Outbound[1]); + } + + [Fact(Timeout = 5000)] + public void OnAttemptFailed_should_exhaust_and_disconnect_once_max_attempts_is_reached() + { + var ops = new FakeClientOps(); + var policy = new ReconnectPolicy(ops, maxAttempts: 2); + policy.Start("buffered-request", MakeTransportOptions()); + + Assert.False(policy.OnAttemptFailed(MakeTransportOptions(), out _)); + var exhausted = policy.OnAttemptFailed(MakeTransportOptions(), out var buffered); + + Assert.True(exhausted); + Assert.Equal("buffered-request", buffered); + Assert.Equal(0, policy.Attempts); + Assert.IsType(ops.Outbound[^1]); + } + + [Fact(Timeout = 5000)] + public void CanReconnect_should_reflect_whether_max_attempts_is_positive() + { + Assert.True(new ReconnectPolicy(new FakeClientOps(), maxAttempts: 1).CanReconnect); + Assert.False(new ReconnectPolicy(new FakeClientOps(), maxAttempts: 0).CanReconnect); + } +} diff --git a/src/GaudiHTTP.Tests/Protocol/Syntax/Http11/Server/Http11ServerBufferedResponseCoalesceSpec.cs b/src/GaudiHTTP.Tests/Protocol/Syntax/Http11/Server/Http11ServerBufferedResponseCoalesceSpec.cs index fd6550f1..71456807 100644 --- a/src/GaudiHTTP.Tests/Protocol/Syntax/Http11/Server/Http11ServerBufferedResponseCoalesceSpec.cs +++ b/src/GaudiHTTP.Tests/Protocol/Syntax/Http11/Server/Http11ServerBufferedResponseCoalesceSpec.cs @@ -94,4 +94,18 @@ public void Chunked_buffered_response_is_not_coalesced() Assert.True(ops.Outbound.Count > 1); } + + [Fact(Timeout = 5000)] + public void Buffered_coalesced_response_rearms_keep_alive_timer() + { + var ops = new FakeServerOps(); + var sm = CreateSm(ops); + SendRequest(sm); + + // Coalesced completion runs through the CompleteResponse epilogue synchronously — + // no pump/body message round-trip is involved on this path. + sm.OnResponse(BufferedResponse("xyz"u8.ToArray(), withContentLength: true)); + + Assert.Contains(ops.ScheduledTimers, t => t.Name == "keep-alive"); + } } diff --git a/src/GaudiHTTP.Tests/Protocol/Syntax/Http11/Server/Http11ServerStateMachineTimerSpec.cs b/src/GaudiHTTP.Tests/Protocol/Syntax/Http11/Server/Http11ServerStateMachineTimerSpec.cs index f40f8aec..0b474433 100644 --- a/src/GaudiHTTP.Tests/Protocol/Syntax/Http11/Server/Http11ServerStateMachineTimerSpec.cs +++ b/src/GaudiHTTP.Tests/Protocol/Syntax/Http11/Server/Http11ServerStateMachineTimerSpec.cs @@ -220,4 +220,44 @@ public void Cleanup_should_cancel_all_timers() Assert.Contains(ops.CancelledTimers, t => t == "request-headers"); Assert.Contains(ops.CancelledTimers, t => t == "keep-alive"); } + + private static (IFeatureCollection Features, GaudiHttpResponseBodyFeature BodyFeature) + CreateStreamingResponseContext() + { + var features = new GaudiFeatureCollection(); + features.Set(new GaudiHttpRequestFeature()); + features.Set(new GaudiHttpResponseFeature { StatusCode = 200 }); + + var bodyFeature = new GaudiHttpResponseBodyFeature(); + var writer = bodyFeature.Writer; + var span = writer.GetSpan(16); + span[..16].Fill(0xAB); + writer.Advance(16); + bodyFeature.UpgradeToPipe(); + + features.Set(bodyFeature); + return (features, bodyFeature); + } + + [Fact(Timeout = 5000)] + [Trait("RFC", "RFC9112-9.3")] + public void OnDrainFailed_should_not_schedule_keep_alive_timer() + { + var ops = new FakeServerOps(); + var sm = new Http11ServerStateMachine(new GaudiServerOptions().ToHttp1Options(), new GaudiServerOptions().ToHttp2Options(), ops); + + var requestData = "GET / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 0\r\n\r\n"; + sm.DecodeClientData(TransportData.Rent(MakeBuffer(requestData))); + + // Writer left incomplete: the pump goes async, so a subsequent read failure hits the + // non-orphaned OnDrainFailed path rather than a normal completion. + var (context, _) = CreateStreamingResponseContext(); + sm.OnResponse(context); + + // A mid-stream drain failure must not rearm the keep-alive timer: the connection's + // response framing is left inconsistent, so it is torn down rather than kept alive. + ((IBodyDrainTarget)sm).OnDrainFailed(0, new IOException("simulated read failure")); + + Assert.DoesNotContain(ops.ScheduleTimerCalls, t => t.Name == "keep-alive"); + } } \ No newline at end of file diff --git a/src/GaudiHTTP.Tests/Protocol/Syntax/Http2/Client/Decoder/Http2StreamStateSpec.cs b/src/GaudiHTTP.Tests/Protocol/Syntax/Http2/Client/Decoder/Http2StreamStateSpec.cs index 8aeee2ae..07af99be 100644 --- a/src/GaudiHTTP.Tests/Protocol/Syntax/Http2/Client/Decoder/Http2StreamStateSpec.cs +++ b/src/GaudiHTTP.Tests/Protocol/Syntax/Http2/Client/Decoder/Http2StreamStateSpec.cs @@ -56,43 +56,6 @@ public void GetResponse_should_throw_when_no_response_initialized() Assert.Throws(state.GetResponse); } - [Fact(Timeout = 5000)] - [Trait("RFC", "RFC9113-5.1")] - public void GetOrCreateResponse_should_return_existing_response() - { - var state = new StreamState(); - var response = new HttpResponseMessage(); - state.InitResponse(response); - - var retrieved = state.GetOrCreateResponse(); - - Assert.Same(response, retrieved); - } - - [Fact(Timeout = 5000)] - [Trait("RFC", "RFC9113-5.1")] - public void GetOrCreateResponse_should_create_response_if_none_exists() - { - var state = new StreamState(); - - var response = state.GetOrCreateResponse(); - - Assert.NotNull(response); - Assert.True(state.HasResponse); - } - - [Fact(Timeout = 5000)] - [Trait("RFC", "RFC9113-5.1")] - public void GetOrCreateResponse_should_return_same_instance_on_multiple_calls() - { - var state = new StreamState(); - - var first = state.GetOrCreateResponse(); - var second = state.GetOrCreateResponse(); - - Assert.Same(first, second); - } - [Fact(Timeout = 5000)] [Trait("RFC", "RFC9113-5.1")] public void AddContentHeader_should_store_header() @@ -311,20 +274,6 @@ public void ContentHeaders_should_not_interfere_with_buffers() Assert.Equal(3, span.Length); } - [Fact(Timeout = 5000)] - [Trait("RFC", "RFC9113-5.1")] - public void GetOrCreateResponse_should_create_once_and_reuse() - { - var state = new StreamState(); - - var resp1 = state.GetOrCreateResponse(); - var resp2 = state.GetOrCreateResponse(); - var resp3 = state.GetOrCreateResponse(); - - Assert.Same(resp1, resp2); - Assert.Same(resp2, resp3); - } - [Fact(Timeout = 5000)] [Trait("RFC", "RFC9113-6.10")] public void AppendHeader_should_reject_block_exceeding_max_accumulated_bytes() diff --git a/src/GaudiHTTP.Tests/Protocol/Syntax/Http2/Client/Http2StreamStateLifecycleSpec.cs b/src/GaudiHTTP.Tests/Protocol/Syntax/Http2/Client/Http2StreamStateLifecycleSpec.cs new file mode 100644 index 00000000..7ab9de02 --- /dev/null +++ b/src/GaudiHTTP.Tests/Protocol/Syntax/Http2/Client/Http2StreamStateLifecycleSpec.cs @@ -0,0 +1,106 @@ +using GaudiHTTP.Protocol.Syntax.Http2; + +namespace GaudiHTTP.Tests.Protocol.Syntax.Http2.Client; + +public sealed class Http2StreamStateLifecycleSpec +{ + [Fact(Timeout = 5000)] + public void Fresh_state_should_not_be_releasable() + { + var state = new StreamState(); + + // No drain has been activated yet, so the local (outbound) direction is trivially + // "done" by default (see StreamState.IsLocalSendComplete) - a fresh stream reads as + // HalfClosedLocal, not Open, until MarkBodyDrainActive() is called. + Assert.Equal(StreamState.StreamLifecycle.HalfClosedLocal, state.Lifecycle); + Assert.False(state.MayRelease); + } + + [Fact(Timeout = 5000)] + public void Fresh_state_with_active_drain_should_be_open() + { + var state = new StreamState(); + state.MarkBodyDrainActive(); + + Assert.Equal(StreamState.StreamLifecycle.Open, state.Lifecycle); + Assert.False(state.MayRelease); + } + + [Fact(Timeout = 5000)] + public void Remote_done_without_active_drain_should_be_immediately_releasable() + { + var state = new StreamState(); + + var releasable = state.OnRemoteDone(); + + Assert.True(releasable); + Assert.True(state.MayRelease); + Assert.Equal(StreamState.StreamLifecycle.Closed, state.Lifecycle); + } + + [Fact(Timeout = 5000)] + public void Local_done_without_active_drain_should_be_immediately_releasable_once_remote_is_also_done() + { + var state = new StreamState(); + + // No drain was ever activated - OnLocalDone reflects a synchronously-completed send that + // never needed the pump (e.g. the whole body fit inline). + var releasableBeforeRemote = state.OnLocalDone(); + Assert.False(releasableBeforeRemote); + Assert.Equal(StreamState.StreamLifecycle.HalfClosedLocal, state.Lifecycle); + + var releasableAfterRemote = state.OnRemoteDone(); + Assert.True(releasableAfterRemote); + Assert.Equal(StreamState.StreamLifecycle.Closed, state.Lifecycle); + } + + [Fact(Timeout = 5000)] + public void Active_drain_should_block_release_until_drain_completes_even_after_remote_closes() + { + var state = new StreamState(); + state.MarkBodyDrainActive(); + + // Remote closes (response fully received) while the request body is still draining - + // must NOT be releasable yet: HalfClosedRemote, not Closed. + var releasableOnRemoteDone = state.OnRemoteDone(); + Assert.False(releasableOnRemoteDone); + Assert.False(state.MayRelease); + Assert.Equal(StreamState.StreamLifecycle.HalfClosedRemote, state.Lifecycle); + + // Drain completes afterward - now both directions are settled. + var releasableOnLocalDone = state.OnLocalDone(); + Assert.True(releasableOnLocalDone); + Assert.True(state.MayRelease); + Assert.Equal(StreamState.StreamLifecycle.Closed, state.Lifecycle); + } + + [Fact(Timeout = 5000)] + public void Active_drain_completing_before_remote_closes_should_block_release_until_remote_closes() + { + var state = new StreamState(); + state.MarkBodyDrainActive(); + + var releasableOnLocalDone = state.OnLocalDone(); + Assert.False(releasableOnLocalDone); + Assert.Equal(StreamState.StreamLifecycle.HalfClosedLocal, state.Lifecycle); + + var releasableOnRemoteDone = state.OnRemoteDone(); + Assert.True(releasableOnRemoteDone); + Assert.Equal(StreamState.StreamLifecycle.Closed, state.Lifecycle); + } + + [Fact(Timeout = 5000)] + public void Reset_should_return_lifecycle_to_open() + { + var state = new StreamState(); + state.MarkBodyDrainActive(); + state.OnRemoteDone(); + state.OnLocalDone(); + Assert.Equal(StreamState.StreamLifecycle.Closed, state.Lifecycle); + + state.Reset(); + + Assert.Equal(StreamState.StreamLifecycle.HalfClosedLocal, state.Lifecycle); + Assert.False(state.MayRelease); + } +} diff --git a/src/GaudiHTTP.Tests/Protocol/Syntax/Http3/Http3OutboundWriterSpec.cs b/src/GaudiHTTP.Tests/Protocol/Syntax/Http3/Http3OutboundWriterSpec.cs new file mode 100644 index 00000000..f816059e --- /dev/null +++ b/src/GaudiHTTP.Tests/Protocol/Syntax/Http3/Http3OutboundWriterSpec.cs @@ -0,0 +1,212 @@ +using Akka.Actor; +using Servus.Akka.Transport; +using GaudiHTTP.Protocol.Body; +using GaudiHTTP.Protocol.Syntax.Http3; + +namespace GaudiHTTP.Tests.Protocol.Syntax.Http3; + +public sealed class Http3OutboundWriterSpec +{ + private sealed class FakeTarget : IMultiplexedBodyDrainTarget + { + public List<(long StreamId, byte[] Data, bool EndStream)> Emitted { get; } = []; + public List Completed { get; } = []; + public List<(long StreamId, Exception Reason)> Failed { get; } = []; + public List PendingMessages { get; } = []; + public IActorRef StageActor => _interceptor; + private readonly MessageInterceptor _interceptor; + + public FakeTarget() + { + _interceptor = new MessageInterceptor(PendingMessages); + } + + public void EmitDataFrames(long streamId, ReadOnlyMemory data, bool endStream) + { + Emitted.Add((streamId, data.ToArray(), endStream)); + } + + public void OnDrainComplete(long streamId) => Completed.Add(streamId); + public void OnDrainFailed(long streamId, Exception reason) => Failed.Add((streamId, reason)); + } + + private sealed class MessageInterceptor : MinimalActorRef + { + private readonly List _messages; + public MessageInterceptor(List messages) => _messages = messages; + public override ActorPath Path { get; } = new RootActorPath(new Address("akka", "test")) / "fake-mux"; + public override IActorRefProvider Provider => throw new NotSupportedException(); + protected override void TellInternal(object message, IActorRef sender) => _messages.Add(message); + } + + private static MemoryStream MakeBody(int size) + { + var data = new byte[size]; + for (var i = 0; i < size; i++) + { + data[i] = (byte)(i % 256); + } + + return new MemoryStream(data); + } + + private static void DriveReadsWithoutCapacity( + Http3OutboundWriter writer, FakeTarget target, int maxIterations = 10_000) + { + var iterations = 0; + while (target.PendingMessages.Count > 0 && iterations++ < maxIterations) + { + var msg = target.PendingMessages[0]; + target.PendingMessages.RemoveAt(0); + switch (msg) + { + case BodyReadComplete rc: + writer.HandleReadComplete(rc.StreamId, rc.BytesRead); + break; + } + } + } + + private static void DrainToCompletion( + Http3OutboundWriter writer, FakeTarget target, int expectedCompletions = 1, int maxIterations = 10_000) + { + var iterations = 0; + while (target.Completed.Count < expectedCompletions + && target.Failed.Count == 0 + && iterations++ < maxIterations) + { + writer.OnCapacityAvailable(); + + if (target.PendingMessages.Count == 0) + { + break; + } + + var msg = target.PendingMessages[0]; + target.PendingMessages.RemoveAt(0); + switch (msg) + { + case BodyReadComplete rc: + writer.HandleReadComplete(rc.StreamId, rc.BytesRead); + break; + } + } + } + + [Fact(Timeout = 5000)] + public void BuildControlPreface_should_produce_valid_wire_format_with_configured_settings() + { + var preface = Http3OutboundWriter.BuildControlPreface( + qpackMaxTableCapacity: 4096, qpackBlockedStreams: 16, maxFieldSectionSize: 65536); + + try + { + Assert.Equal(CriticalStreamId.Control, preface.StreamId); + + var span = preface.Buffer.Memory.Span; + var streamType = QuicVarInt.Decode(span, out var read); + Assert.Equal((long)StreamType.Control, streamType); + + var decoder = new FrameDecoder(); + var frames = decoder.DecodeAll(span[read..], out _); + var settingsFrame = Assert.Single(frames.OfType()); + + var parsed = settingsFrame.Parameters.ToDictionary(p => p.Identifier, p => p.Value); + Assert.Equal(4096, parsed[SettingsIdentifier.QpackMaxTableCapacity]); + Assert.Equal(16, parsed[SettingsIdentifier.QpackBlockedStreams]); + Assert.Equal(65536, parsed[SettingsIdentifier.MaxFieldSectionSize]); + } + finally + { + preface.Buffer.Dispose(); + } + } + + [Fact(Timeout = 5000)] + public void EmitDataFrame_should_produce_valid_H3_DATA_frame_wire_format() + { + var body = new byte[128]; + new Random(7).NextBytes(body); + var emitted = new List(); + + Http3OutboundWriter.EmitDataFrame(emitted.Add, streamId: 4, body); + + var item = Assert.Single(emitted.OfType()); + Assert.Equal(4, item.StreamId); + + var decoder = new FrameDecoder(); + var frames = decoder.DecodeAll(item.Buffer.Memory.Span, out _); + var dataFrame = Assert.Single(frames.OfType()); + Assert.Equal(body, dataFrame.Data.ToArray()); + + item.Buffer.Dispose(); + } + + [Fact(Timeout = 5000)] + public void EmitDataFrame_should_be_noop_for_empty_body() + { + var emitted = new List(); + + Http3OutboundWriter.EmitDataFrame(emitted.Add, streamId: 1, ReadOnlyMemory.Empty); + + Assert.Empty(emitted); + } + + [Fact(Timeout = 5000)] + public void Register_should_drain_body_and_emit_endStream_when_capacity_is_granted() + { + var target = new FakeTarget(); + var writer = new Http3OutboundWriter(target, new CancellationTokenSource(), bodyChunkSize: 16 * 1024); + + writer.Register(1L, MakeBody(100), CancellationToken.None); + DrainToCompletion(writer, target); + + Assert.Equal(2, target.Emitted.Count); + Assert.Equal(100, target.Emitted[0].Data.Length); + Assert.False(target.Emitted[0].EndStream); + Assert.True(target.Emitted[1].EndStream); + Assert.Single(target.Completed); + } + + [Fact(Timeout = 5000)] + public void Register_should_bound_in_flight_emissions_to_OutboundBodyCapacity_without_drain_signal() + { + var target = new FakeTarget(); + // Body large enough to need far more chunks than the credit cap allows. + var writer = new Http3OutboundWriter(target, new CancellationTokenSource(), bodyChunkSize: 16); + + writer.Register(1L, MakeBody(16 * (Http3OutboundWriter.OutboundBodyCapacity * 4)), CancellationToken.None); + DriveReadsWithoutCapacity(writer, target); + + var emittedData = target.Emitted.Count(e => !e.EndStream); + Assert.True( + emittedData <= Http3OutboundWriter.OutboundBodyCapacity, + $"expected at most {Http3OutboundWriter.OutboundBodyCapacity} in-flight emissions without a drain signal, got {emittedData}"); + Assert.Empty(target.Completed); + } + + [Fact(Timeout = 5000)] + public void HandleReadFailed_should_report_failure_and_Cancel_should_be_a_noop_after_completion() + { + var target = new FakeTarget(); + var writer = new Http3OutboundWriter(target, new CancellationTokenSource(), bodyChunkSize: 16 * 1024); + + writer.Register(1L, MakeBody(10), CancellationToken.None); + DrainToCompletion(writer, target); + Assert.Single(target.Completed); + + writer.Cancel(1L); + Assert.Single(target.Completed); + Assert.Empty(target.Failed); + } + + [Fact(Timeout = 5000)] + public void Cleanup_should_be_idempotent_and_safe_before_any_Register() + { + var target = new FakeTarget(); + var writer = new Http3OutboundWriter(target, new CancellationTokenSource(), bodyChunkSize: 16 * 1024); + + writer.Cleanup(); + writer.Cleanup(); + } +} diff --git a/src/GaudiHTTP.Tests/Streams/Stages/Lifecycle/ServerSupervisorActorSpec.cs b/src/GaudiHTTP.Tests/Streams/Stages/Lifecycle/ServerSupervisorActorSpec.cs index 44bdce88..50900ebe 100644 --- a/src/GaudiHTTP.Tests/Streams/Stages/Lifecycle/ServerSupervisorActorSpec.cs +++ b/src/GaudiHTTP.Tests/Streams/Stages/Lifecycle/ServerSupervisorActorSpec.cs @@ -45,6 +45,24 @@ public Source, Task> B } } + private sealed class NeverCompletingListenerFactory : IListenerFactory + { + private readonly int _boundPort; + + public NeverCompletingListenerFactory(int boundPort) + { + _boundPort = boundPort; + } + + public Source, Task> Bind(ListenerOptions options) + { + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + tcs.SetResult(_boundPort); + return Source.Maybe>() + .MapMaterializedValue(_ => tcs.Task); + } + } + private static ServerSupervisorActor.StartServer StartMsg(params ListenerBinding[] bindings) => new(PassthroughBridge(), new GaudiServerOptions(), bindings); @@ -83,6 +101,39 @@ public void Start_should_reply_ListenersFailed_when_any_listener_dies() Assert.NotNull(failed.Error); } + [Fact(Timeout = 5000)] + public async Task Sibling_listeners_should_be_stopped_when_one_listener_dies_during_startup() + { + var supervisor = Sys.ActorOf(Props.Create(() => new ServerSupervisorActor())); + + supervisor.Tell(StartMsg( + new ListenerBinding + { + Factory = new FailingListenerFactory(), + Options = new TcpListenerOptions { Host = "localhost", Port = 0 } + }, + new ListenerBinding + { + Factory = new NeverCompletingListenerFactory(9001), + Options = new TcpListenerOptions { Host = "localhost", Port = 0 } + }), TestActor); + + ExpectMsg( + cancellationToken: TestContext.Current.CancellationToken); + + try + { + var sibling = await Sys.ActorSelection(supervisor.Path / "listener-1") + .ResolveOne(TimeSpan.FromSeconds(1)); + Watch(sibling); + ExpectTerminated(sibling, cancellationToken: TestContext.Current.CancellationToken); + } + catch (ActorNotFoundException) + { + // Sibling was already stopped before we could resolve it — desired outcome. + } + } + [Fact(Timeout = 5000)] public void Start_should_reply_ListenersReady_immediately_when_no_bindings() { diff --git a/src/GaudiHTTP/Pooling/ConnectionObjectPool.cs b/src/GaudiHTTP/Pooling/ConnectionObjectPool.cs index 09881ddd..0aa04187 100644 --- a/src/GaudiHTTP/Pooling/ConnectionObjectPool.cs +++ b/src/GaudiHTTP/Pooling/ConnectionObjectPool.cs @@ -14,26 +14,30 @@ internal sealed class ConnectionObjectPool public T Rent(Func factory) where T : class, IResetable { - var obj = GetPool(factory).Get(); + var holder = GetHolder(); + holder.Policy.EnsureFactory(factory); + var obj = holder.Pool.Get(); obj.OnRented(); return obj; } + // Must never require a factory: pooled objects self-return on Dispose, and the first pool + // touch for a type may be a Return (an object constructed directly rather than rented). + // The factory is latched later by the first Rent. public void Return(T obj) where T : class, IResetable + => GetHolder().Pool.Return(obj); + + private PoolHolder GetHolder() where T : class, IResetable + => (PoolHolder)_pools.GetOrAdd(typeof(T), static _ => new PoolHolder()); + + private sealed class PoolHolder where T : class, IResetable { - // No Rent has created the pool yet (e.g. a directly-constructed instance is disposed). - // Treat it like returning to a full pool: reset and discard. - if (_pools.TryGetValue(typeof(T), out var pool)) - { - ((ObjectPool)pool).Return(obj); - } - else + public readonly ResettablePoolPolicy Policy = new(null); + public readonly ObjectPool Pool; + + public PoolHolder() { - obj.Reset(); + Pool = new DefaultObjectPool(Policy, maximumRetained: 256); } } - - private ObjectPool GetPool(Func factory) where T : class, IResetable - => (ObjectPool)_pools.GetOrAdd(typeof(T), - _ => new DefaultObjectPool(new ResettablePoolPolicy(factory), maximumRetained: 256)); } diff --git a/src/GaudiHTTP/Pooling/ResettablePoolPolicy.cs b/src/GaudiHTTP/Pooling/ResettablePoolPolicy.cs index bd8bf07f..427f4cdc 100644 --- a/src/GaudiHTTP/Pooling/ResettablePoolPolicy.cs +++ b/src/GaudiHTTP/Pooling/ResettablePoolPolicy.cs @@ -3,11 +3,18 @@ namespace GaudiHTTP.Pooling; // Creates instances via an injected factory (handles ctor args) and resets them on return. One -// policy type serves every pooled object kind. -internal sealed class ResettablePoolPolicy(Func factory) : IPooledObjectPolicy +// policy type serves every pooled object kind. The factory is latched on the first Rent rather +// than required at construction, because the first pool touch for a type may be a Return (a +// directly constructed object disposing itself back into the pool) — see ConnectionObjectPool. +internal sealed class ResettablePoolPolicy(Func? factory) : IPooledObjectPolicy where T : class, IResetable { - public T Create() => factory(); + private Func? _factory = factory; + + public void EnsureFactory(Func create) => _factory ??= create; + + public T Create() => (_factory ?? throw new InvalidOperationException( + $"No factory registered for pooled type {typeof(T).Name}."))(); public bool Return(T obj) { diff --git a/src/GaudiHTTP/Protocol/Body/DrainMessages.cs b/src/GaudiHTTP/Protocol/Body/DrainMessages.cs index d61a4db7..cbfcf251 100644 --- a/src/GaudiHTTP/Protocol/Body/DrainMessages.cs +++ b/src/GaudiHTTP/Protocol/Body/DrainMessages.cs @@ -4,4 +4,3 @@ namespace GaudiHTTP.Protocol.Body; // its natural key with no casts: H1/H2 use int (H1 always 0), H3 uses long (QUIC stream ids). internal readonly record struct BodyReadComplete(TStreamId StreamId, int BytesRead); internal readonly record struct BodyReadFailed(TStreamId StreamId, Exception Reason); -internal readonly record struct BodyReadContinue(TStreamId StreamId); diff --git a/src/GaudiHTTP/Protocol/Body/FlowControlledBodyPump.cs b/src/GaudiHTTP/Protocol/Body/FlowControlledBodyPump.cs index 44c4ad54..30dec58c 100644 --- a/src/GaudiHTTP/Protocol/Body/FlowControlledBodyPump.cs +++ b/src/GaudiHTTP/Protocol/Body/FlowControlledBodyPump.cs @@ -1,4 +1,3 @@ -using Akka.Actor; using GaudiHTTP.Pooling; using GaudiHTTP.Protocol.Syntax.Http2; @@ -11,12 +10,10 @@ internal sealed class FlowControlledBodyPump( int chunkSize, int hardCap) { - private const int MaxSyncReadsPerDispatch = 64; - private readonly Queue _readyQueue = new(); private readonly Dictionary> _activeSlots = new(); - private readonly HashSet _cancelledStreams = new(); private readonly HashSet _windowBlockedStreams = new(); + private readonly List _stillBlockedScratch = new(); private int _readSlots = 2; private int _asyncInFlight; @@ -50,7 +47,7 @@ public void OnWindowUpdate(int streamId) { if (flowController.ConnectionSendWindow >= minRead) { - var stillBlocked = new List(); + _stillBlockedScratch.Clear(); foreach (var blocked in _windowBlockedStreams) { if (flowController.GetStreamSendWindow(blocked) >= minRead) @@ -59,12 +56,12 @@ public void OnWindowUpdate(int streamId) } else { - stillBlocked.Add(blocked); + _stillBlockedScratch.Add(blocked); } } _windowBlockedStreams.Clear(); - foreach (var id in stillBlocked) + foreach (var id in _stillBlockedScratch) { _windowBlockedStreams.Add(id); } @@ -97,9 +94,7 @@ public void HandleReadComplete(int streamId, int bytesRead) slot.ReservedWindow = 0; } - slot.DisposeResources(); - slot.Dispose(); - _activeSlots.Remove(streamId); + PumpSlotLifecycle.ReleaseSlot(_activeSlots, streamId, slot); return; } @@ -125,39 +120,22 @@ public void HandleReadFailed(int streamId, Exception reason) slot.ReservedWindow = 0; } - slot.DisposeResources(); - slot.Dispose(); - _activeSlots.Remove(streamId); + PumpSlotLifecycle.ReleaseSlot(_activeSlots, streamId, slot); return; } + if (slot.ReservedWindow > 0) + { + flowController.Refund(slot.StreamId, slot.ReservedWindow); + slot.ReservedWindow = 0; + } + _activeSlots.Remove(streamId); target.OnDrainFailed(streamId, reason); slot.DisposeResources(); slot.Dispose(); } - public void HandleBodyReadContinue(int streamId) - { - if (!_activeSlots.TryGetValue(streamId, out var slot)) - { - return; - } - - slot.ResetSyncReads(); - - if (slot.IsOrphaned) - { - slot.DisposeResources(); - slot.Dispose(); - _activeSlots.Remove(streamId); - return; - } - - _readyQueue.Enqueue(streamId); - TryScheduleReads(); - } - public void Cancel(int streamId) { if (!_activeSlots.TryGetValue(streamId, out var slot)) @@ -173,15 +151,11 @@ public void Cancel(int streamId) return; } - if (_windowBlockedStreams.Remove(streamId)) - { - _activeSlots.Remove(streamId); - slot.DisposeResources(); - slot.Dispose(); - return; - } - - _cancelledStreams.Add(streamId); + // Not in flight — clean up immediately, matching MultiplexedBodyPump's discipline (lazy + // removal from the ready queue is not needed: the slot will simply be missing from + // _activeSlots when dequeued in TryScheduleReads). + _windowBlockedStreams.Remove(streamId); + PumpSlotLifecycle.ReleaseSlot(_activeSlots, streamId, slot); } public void Cleanup() @@ -199,7 +173,6 @@ public void Cleanup() _activeSlots.Clear(); _windowBlockedStreams.Clear(); - _cancelledStreams.Clear(); _readyQueue.Clear(); } @@ -216,18 +189,6 @@ private void TryScheduleReads() { var streamId = _readyQueue.Dequeue(); - if (_cancelledStreams.Remove(streamId)) - { - if (_activeSlots.TryGetValue(streamId, out var cancelled)) - { - _activeSlots.Remove(streamId); - cancelled.DisposeResources(); - cancelled.Dispose(); - } - - continue; - } - if (!_activeSlots.TryGetValue(streamId, out var slot)) { continue; @@ -244,14 +205,6 @@ private void TryScheduleReads() continue; } - if (slot.ConsecutiveSyncReads >= MaxSyncReadsPerDispatch) - { - slot.ResetSyncReads(); - slot.BeginRead(); // marks as in-flight for yield - target.StageActor.Tell(new BodyReadContinue(slot.StreamId), ActorRefs.NoSender); - continue; - } - slot.EnsureBuffer(chunkSize); StartRead(slot); @@ -268,29 +221,7 @@ private void StartRead(PumpSlot slot) slot.ReservedWindow = readSize; var token = slot.LinkedCts?.Token ?? connectionCts.Token; - slot.BeginRead(); - var vt = slot.BodyStream!.ReadAsync(slot.Buffer!.Memory[..readSize], token); - - if (vt.IsCompletedSuccessfully) - { - // Force-async: identical to the PipeTo path below but delivering the already-known - // result. The slot stays IsReadInFlight (from BeginRead) and counted in _asyncInFlight - // across the mailbox hop so HandleReadComplete's CompleteRead/_asyncInFlight-- balance - // and no re-entrant schedule can touch slot.Buffer before the completion emits it. - slot.ResetSyncReads(); - _asyncInFlight++; - target.StageActor.Tell( - slot.CachedSuccessTransform!(vt.Result), - ActorRefs.NoSender); - return; - } - - slot.ResetSyncReads(); - _asyncInFlight++; - vt.PipeTo( - target.StageActor, - success: slot.CachedSuccessTransform, - failure: slot.CachedFailureTransform); + PumpSlotLifecycle.StartRead(slot, readSize, token, target.StageActor, ref _asyncInFlight); } private void ProcessReadResult(PumpSlot slot, int bytesRead) diff --git a/src/GaudiHTTP/Protocol/Body/MultiplexedBodyPump.cs b/src/GaudiHTTP/Protocol/Body/MultiplexedBodyPump.cs index 8ef946c0..eebe14e8 100644 --- a/src/GaudiHTTP/Protocol/Body/MultiplexedBodyPump.cs +++ b/src/GaudiHTTP/Protocol/Body/MultiplexedBodyPump.cs @@ -1,4 +1,3 @@ -using Akka.Actor; using GaudiHTTP.Pooling; namespace GaudiHTTP.Protocol.Body; @@ -10,8 +9,6 @@ internal sealed class MultiplexedBodyPump( int maxCapacity, int maxConcurrentReads = 4) { - private const int MaxSyncReadsPerDispatch = 64; - private readonly Queue _readyQueue = new(); private readonly Dictionary> _activeSlots = new(); @@ -50,9 +47,7 @@ public void HandleReadComplete(long streamId, int bytesRead) if (slot.IsOrphaned) { - slot.DisposeResources(); - slot.Dispose(); - _activeSlots.Remove(streamId); + PumpSlotLifecycle.ReleaseSlot(_activeSlots, streamId, slot); return; } @@ -72,9 +67,7 @@ public void HandleReadFailed(long streamId, Exception reason) if (slot.IsOrphaned) { - slot.DisposeResources(); - slot.Dispose(); - _activeSlots.Remove(streamId); + PumpSlotLifecycle.ReleaseSlot(_activeSlots, streamId, slot); return; } @@ -94,28 +87,6 @@ public void OnCapacityAvailable() TryScheduleReads(); } - public void HandleBodyReadContinue(long streamId) - { - if (!_activeSlots.TryGetValue(streamId, out var slot)) - { - return; - } - - // Clear counter and in-flight marker set by starvation guard - slot.ResetSyncReads(); - - if (slot.IsOrphaned) - { - slot.DisposeResources(); - slot.Dispose(); - _activeSlots.Remove(streamId); - return; - } - - _readyQueue.Enqueue(streamId); - TryScheduleReads(); - } - public void Cancel(long streamId) { if (!_activeSlots.TryGetValue(streamId, out var slot)) @@ -134,9 +105,7 @@ public void Cancel(long streamId) // Not in flight — clean up immediately (lazy removal from ready queue is not needed // since the slot will simply be missing from _activeSlots when dequeued) - _activeSlots.Remove(streamId); - slot.DisposeResources(); - slot.Dispose(); + PumpSlotLifecycle.ReleaseSlot(_activeSlots, streamId, slot); } public void Cleanup() @@ -168,19 +137,6 @@ private void TryScheduleReads() continue; } - // Starvation guard: yield after MaxSyncReadsPerDispatch consecutive sync reads so - // the actor thread can process other messages between bursts. This yield does not emit - // a frame, so it must not consume outbound credit. - if (slot.ConsecutiveSyncReads >= MaxSyncReadsPerDispatch) - { - slot.ResetSyncReads(); - // Use BeginRead as a yield-in-progress marker so re-entrant calls from - // ProcessReadResult cannot start another read while waiting for HandleBodyReadContinue. - slot.BeginRead(); - target.StageActor.Tell(new BodyReadContinue(slot.StreamId), ActorRefs.NoSender); - continue; - } - _availableCapacity--; slot.EnsureBuffer(chunkSize); @@ -191,29 +147,7 @@ private void TryScheduleReads() private void StartRead(PumpSlot slot) { var token = slot.LinkedCts?.Token ?? connectionCts.Token; - slot.BeginRead(); - var vt = slot.BodyStream!.ReadAsync(slot.Buffer!.Memory[..chunkSize], token); - - if (vt.IsCompletedSuccessfully) - { - // Force-async: identical to the PipeTo path below but delivering the already-known - // result. The slot stays IsReadInFlight (from BeginRead) and counted in _asyncInFlight - // across the mailbox hop so HandleReadComplete's CompleteRead/_asyncInFlight-- balance - // and no re-entrant schedule can touch slot.Buffer before the completion emits it. - slot.ResetSyncReads(); - _asyncInFlight++; - target.StageActor.Tell( - slot.CachedSuccessTransform!(vt.Result), - ActorRefs.NoSender); - return; - } - - slot.ResetSyncReads(); - _asyncInFlight++; - vt.PipeTo( - target.StageActor, - success: slot.CachedSuccessTransform, - failure: slot.CachedFailureTransform); + PumpSlotLifecycle.StartRead(slot, chunkSize, token, target.StageActor, ref _asyncInFlight); } private void ProcessReadResult(PumpSlot slot, int bytesRead) diff --git a/src/GaudiHTTP/Protocol/Body/PumpSlot.cs b/src/GaudiHTTP/Protocol/Body/PumpSlot.cs index 30c8c218..f9a03181 100644 --- a/src/GaudiHTTP/Protocol/Body/PumpSlot.cs +++ b/src/GaudiHTTP/Protocol/Body/PumpSlot.cs @@ -13,7 +13,6 @@ internal sealed class PumpSlot : Poolable> public CancellationToken RequestCt { get; private set; } public long? ContentLength { get; set; } public int ReservedWindow { get; set; } - public int ConsecutiveSyncReads { get; private set; } public bool IsReadInFlight { get; private set; } public bool IsOrphaned { get; private set; } @@ -48,21 +47,7 @@ public void EnsureBuffer(int chunkSize) public void BeginRead() => IsReadInFlight = true; - public void CompleteRead() - { - IsReadInFlight = false; - ConsecutiveSyncReads = 0; - } - - public void IncrementSyncReads() => ConsecutiveSyncReads++; - - public void CompleteSyncRead() - { - IsReadInFlight = false; - ConsecutiveSyncReads++; - } - - public void ResetSyncReads() => ConsecutiveSyncReads = 0; + public void CompleteRead() => IsReadInFlight = false; public void MarkOrphaned() => IsOrphaned = true; @@ -83,6 +68,5 @@ protected override void OnReset() ReservedWindow = 0; IsReadInFlight = false; IsOrphaned = false; - ConsecutiveSyncReads = 0; } } diff --git a/src/GaudiHTTP/Protocol/Body/PumpSlotLifecycle.cs b/src/GaudiHTTP/Protocol/Body/PumpSlotLifecycle.cs new file mode 100644 index 00000000..282ca83a --- /dev/null +++ b/src/GaudiHTTP/Protocol/Body/PumpSlotLifecycle.cs @@ -0,0 +1,52 @@ +using Akka.Actor; + +namespace GaudiHTTP.Protocol.Body; + +// Static composition helpers shared by the PumpSlot-based pumps (FlowControlledBodyPump, +// MultiplexedBodyPump). SerialBodyPump does not use PumpSlot (it tracks a single stream via +// plain fields), so it is not a candidate for these helpers. Deliberately NOT a base class — a +// BodyPumpBase inheritance unification was tried and reverted; use composition only. +internal static class PumpSlotLifecycle +{ + // Removes the slot from the active-slot map and releases its resources. Shared by every + // orphan/cancel/teardown path that does not need to notify the drain target first. + public static void ReleaseSlot( + Dictionary> activeSlots, + TStreamId streamId, + PumpSlot slot) where TStreamId : notnull + { + activeSlots.Remove(streamId); + slot.DisposeResources(); + slot.Dispose(); + } + + // Starts the read and forces the completion to always flow through the actor mailbox, even + // when the ValueTask is already completed. Delivering the already-known result synchronously + // (instead of dispatching it) would let a re-entrant schedule touch slot.Buffer before the + // "in-flight" completion is processed. The slot stays IsReadInFlight (from BeginRead) and + // counted in asyncInFlight across the mailbox hop so the completion handler's + // CompleteRead/asyncInFlight-- balance always holds. + public static void StartRead( + PumpSlot slot, + int readSize, + CancellationToken token, + IActorRef stageActor, + ref int asyncInFlight) + { + slot.BeginRead(); + var vt = slot.BodyStream!.ReadAsync(slot.Buffer!.Memory[..readSize], token); + + if (vt.IsCompletedSuccessfully) + { + asyncInFlight++; + stageActor.Tell(slot.CachedSuccessTransform!(vt.Result), ActorRefs.NoSender); + return; + } + + asyncInFlight++; + vt.PipeTo( + stageActor, + success: slot.CachedSuccessTransform, + failure: slot.CachedFailureTransform); + } +} diff --git a/src/GaudiHTTP/Protocol/Body/SerialBodyPump.cs b/src/GaudiHTTP/Protocol/Body/SerialBodyPump.cs index 2496d941..4b061a2a 100644 --- a/src/GaudiHTTP/Protocol/Body/SerialBodyPump.cs +++ b/src/GaudiHTTP/Protocol/Body/SerialBodyPump.cs @@ -10,14 +10,11 @@ internal sealed class SerialBodyPump( int chunkSize, int maxCapacity) { - private const int MaxSyncReadsPerDispatch = 64; - private Stream? _activeStream; private IMemoryOwner? _activeOwner; private CancellationTokenSource? _linkedCts; private bool _isReadInFlight; private int _availableCapacity; - private int _consecutiveSyncReads; // Serial stream id is always 0, so the read-completion transforms capture nothing and are // shared statically — no per-Register closure allocation. @@ -31,7 +28,6 @@ public void Register(Stream bodyStream, long? contentLength, CancellationToken r ? CancellationTokenSource.CreateLinkedTokenSource(connectionCts.Token, requestCt) : null; _availableCapacity = maxCapacity; - _consecutiveSyncReads = 0; TryStartRead(); } @@ -48,14 +44,12 @@ public void OnCapacityAvailable() public void HandleReadComplete(int bytesRead) { _isReadInFlight = false; - _consecutiveSyncReads = 0; ProcessReadResult(bytesRead); } public void HandleReadFailed(Exception reason) { _isReadInFlight = false; - _consecutiveSyncReads = 0; _activeOwner?.Dispose(); _activeOwner = null; _activeStream = null; @@ -63,12 +57,6 @@ public void HandleReadFailed(Exception reason) CompleteDrain(); } - public void HandleBodyReadContinue() - { - _isReadInFlight = false; - TryStartRead(); - } - public void Cancel() { _linkedCts?.Cancel(); @@ -99,19 +87,6 @@ private void TryStartRead() return; } - // Starvation guard: yield after MaxSyncReadsPerDispatch consecutive sync reads - // so the actor thread can process other messages between bursts. - if (_consecutiveSyncReads >= MaxSyncReadsPerDispatch) - { - _consecutiveSyncReads = 0; - // Use _isReadInFlight as a yield-in-progress marker so that any re-entrant - // call from ProcessReadResult cannot start another read while we wait for - // HandleBodyReadContinue to resume us. - _isReadInFlight = true; - target.StageActor.Tell(new BodyReadContinue(0), ActorRefs.NoSender); - return; - } - _availableCapacity--; var token = _linkedCts?.Token ?? connectionCts.Token; _isReadInFlight = true; @@ -124,12 +99,10 @@ private void TryStartRead() // processed. Keep _isReadInFlight = true across the mailbox hop (exactly like the // PipeTo path below) so an interleaved OnCapacityAvailable -> TryStartRead cannot // start the next read before HandleReadComplete hands off the current owner. - _consecutiveSyncReads = 0; target.StageActor.Tell(CachedSuccess(vt.Result), ActorRefs.NoSender); return; } - _consecutiveSyncReads = 0; vt.PipeTo( target.StageActor, success: CachedSuccess, diff --git a/src/GaudiHTTP/Protocol/Semantics/ConnectionRateGuard.cs b/src/GaudiHTTP/Protocol/Semantics/ConnectionRateGuard.cs new file mode 100644 index 00000000..18f025d6 --- /dev/null +++ b/src/GaudiHTTP/Protocol/Semantics/ConnectionRateGuard.cs @@ -0,0 +1,102 @@ +using GaudiHTTP.Protocol; +using GaudiHTTP.Server; +using GaudiHTTP.Streams.Stages.Server; + +namespace GaudiHTTP.Protocol.Semantics; + +/// +/// Owns the request/response data-rate orchestration cluster shared by the HTTP/1.0 and HTTP/1.1 +/// server state machines: a pair of s (one per direction), the +/// violation list reused across timer firings, and the single-shot "data-rate-check" timer. +/// Callers observe bytes as they flow, and route the connection's timer-fired dispatch through +/// when the timer name matches . +/// +internal sealed class ConnectionRateGuard +{ + public const string TimerName = "data-rate-check"; + + private readonly IServerStageOperations _ops; + private readonly TimeProvider _clock; + private readonly DataRateMonitor _requestRate; + private readonly DataRateMonitor _responseRate; + private readonly List _violations = []; + private bool _timerActive; + + public ConnectionRateGuard(IServerStageOperations ops, DataRateOptions rate, TimeProvider? clock = null) + { + _ops = ops ?? throw new ArgumentNullException(nameof(ops)); + _clock = clock ?? TimeProvider.System; + _requestRate = new DataRateMonitor(rate.MinRequestBodyDataRate, rate.MinRequestBodyDataRateGracePeriod); + _responseRate = new DataRateMonitor(rate.MinResponseDataRate, rate.MinResponseDataRateGracePeriod); + } + + private long Now() => _clock.GetUtcNow().ToUnixTimeMilliseconds(); + + public void ObserveRequest(long streamId, long bytes) + { + _requestRate.Observe(streamId, bytes, Now()); + EnsureTimer(); + } + + public void ObserveResponse(long streamId, long bytes) + { + _responseRate.Observe(streamId, bytes, Now()); + EnsureTimer(); + } + + public void RemoveRequest(long streamId) + { + _requestRate.Remove(streamId); + } + + public void RemoveResponse(long streamId) + { + _responseRate.Remove(streamId); + } + + /// + /// Handles the timer firing: checks both monitors for violations and + /// rearms the timer if either still has active entries. Returns if a + /// violation was detected, in which case (if given) is invoked + /// with the current request/response entry counts so the caller can log before tearing the + /// connection down — the guard itself never rearms after a violation. + /// + public bool OnTimerFired(Action? onViolation = null) + { + _timerActive = false; + _violations.Clear(); + var now = Now(); + _requestRate.Check(now, _violations); + _responseRate.Check(now, _violations); + + if (_violations.Count > 0) + { + onViolation?.Invoke(_requestRate.Count, _responseRate.Count); + return true; + } + + if (_requestRate.Count > 0 || _responseRate.Count > 0) + { + EnsureTimer(); + } + + return false; + } + + public void EnsureTimer() + { + if (_timerActive) + { + return; + } + + _timerActive = true; + _ops.OnScheduleTimer(TimerName, TimeSpan.FromSeconds(1)); + } + + public void Cleanup() + { + _ops.OnCancelTimer(TimerName); + _timerActive = false; + } +} diff --git a/src/GaudiHTTP/Protocol/Semantics/ReconnectPolicy.cs b/src/GaudiHTTP/Protocol/Semantics/ReconnectPolicy.cs new file mode 100644 index 00000000..96ee7bd9 --- /dev/null +++ b/src/GaudiHTTP/Protocol/Semantics/ReconnectPolicy.cs @@ -0,0 +1,80 @@ +using Servus.Akka.Transport; +using GaudiHTTP.Streams.Stages.Client; + +namespace GaudiHTTP.Protocol.Semantics; + +/// +/// Owns the attempt-counting mechanics shared by the HTTP/1.0 and HTTP/1.1 client reconnect +/// trio (start / restore / attempt-failed), generic over whatever work the caller buffers while +/// reconnecting (a single request for HTTP/1.0, a request queue for HTTP/1.1). This class does +/// NOT own connection lifecycle state — callers keep their own state (a bool, or an explicit +/// enum) and only ask this policy "how many attempts are left" / "emit the next transport op". +/// +internal sealed class ReconnectPolicy +{ + private readonly IClientStageOperations _ops; + private readonly int _maxAttempts; + private int _attempts; + private TBuffered? _buffered; + + public ReconnectPolicy(IClientStageOperations ops, int maxAttempts) + { + _ops = ops ?? throw new ArgumentNullException(nameof(ops)); + _maxAttempts = maxAttempts; + } + + public bool CanReconnect => _maxAttempts > 0; + + public int Attempts => _attempts; + + /// Non-destructive peek at the currently buffered work (or default if none). + public TBuffered? Buffered => _buffered; + + /// + /// Begins reconnecting: stashes for later replay/failure, resets + /// the attempt counter to 1, and emits the first . + /// + public void Start(TBuffered buffered, TransportOptions transportOptions) + { + _buffered = buffered; + _attempts = 1; + _ops.OnOutbound(new ConnectTransport(transportOptions)); + } + + /// + /// Takes ownership of whatever was buffered, resetting the attempt counter. Used both when a + /// reconnect succeeds (caller replays the buffered work) and when it is abandoned outright + /// (caller fails the buffered work) — the two cases are distinguished by what the caller does + /// with the returned value, not by this method. + /// + public TBuffered? TakeBuffered() + { + var buffered = _buffered; + _buffered = default; + _attempts = 0; + return buffered; + } + + /// + /// Records a failed reconnect attempt. If is now exhausted, takes + /// ownership of the buffered work (via ) and emits a + /// , returning so the caller can fail + /// the buffered work and move its own lifecycle state to "dead". Otherwise increments the + /// attempt counter and emits the next , returning + /// . + /// + public bool OnAttemptFailed(TransportOptions transportOptions, out TBuffered? buffered) + { + if (_attempts >= _maxAttempts) + { + buffered = TakeBuffered(); + _ops.OnOutbound(new DisconnectTransport(DisconnectReason.Error)); + return true; + } + + buffered = default; + _attempts++; + _ops.OnOutbound(new ConnectTransport(transportOptions)); + return false; + } +} diff --git a/src/GaudiHTTP/Protocol/Syntax/Http10/Client/Http10ClientStateMachine.cs b/src/GaudiHTTP/Protocol/Syntax/Http10/Client/Http10ClientStateMachine.cs index e30f7846..87a875f6 100644 --- a/src/GaudiHTTP/Protocol/Syntax/Http10/Client/Http10ClientStateMachine.cs +++ b/src/GaudiHTTP/Protocol/Syntax/Http10/Client/Http10ClientStateMachine.cs @@ -4,6 +4,7 @@ using GaudiHTTP.Client; using GaudiHTTP.Internal; using GaudiHTTP.Protocol.Body; +using GaudiHTTP.Protocol.Semantics; using GaudiHTTP.Streams.Stages.Client; using static Servus.Senf; @@ -17,8 +18,7 @@ internal sealed class Http10ClientStateMachine : IClientStateMachine, IBodyDrain private readonly GaudiClientOptions _options; private TransportOptions? _transportOptions; private HttpRequestMessage? _inFlightRequest; - private HttpRequestMessage? _reconnectBufferedRequest; - private int _reconnectAttempts; + private readonly ReconnectPolicy _reconnectPolicy; private bool _lastRequestWasHead; private bool _outboundBodyPending; private IStreamingBodyReader? _activeStreamingReader; @@ -43,7 +43,7 @@ private int PendingRequestCount { if (IsReconnecting) { - return _reconnectBufferedRequest is not null ? 1 : 0; + return _reconnectPolicy.Buffered is not null ? 1 : 0; } return (_inFlightRequest is not null || _outboundBodyPending) ? 1 : 0; @@ -56,6 +56,7 @@ public Http10ClientStateMachine(GaudiClientOptions options, IClientStageOperatio { _ops = ops; _options = options; + _reconnectPolicy = new ReconnectPolicy(ops, options.Http1.MaxReconnectAttempts); var decoderOpts = options.ToHttp10DecoderOptions(); @@ -178,19 +179,17 @@ public void OnUpstreamFinished() if (IsReconnecting) { - if (_reconnectBufferedRequest is { } buffered) + if (_reconnectPolicy.TakeBuffered() is { } buffered) { buffered.Fail(new HttpRequestException("HTTP/1.0 transport closed during reconnect.")); - _reconnectBufferedRequest = null; } IsReconnecting = false; - _reconnectAttempts = 0; Tracing.For("Protocol").Debug(this, "HTTP/1.0 transport closed during reconnect"); return; } - TryDecodeEof(bodyComplete); + TryCompleteAfterEof(bodyComplete); FailOrphanedRequest(); } @@ -212,10 +211,6 @@ public void OnBodyMessage(object msg) case BodyReadFailed failed: _serialPump?.HandleReadFailed(failed.Reason); break; - - case BodyReadContinue: - _serialPump?.HandleBodyReadContinue(); - break; } } @@ -373,7 +368,7 @@ private void HandleDisconnect(TransportDisconnected disconnect) return; } - if (HasInFlightRequests && _options.Http1.MaxReconnectAttempts > 0) + if (HasInFlightRequests && _reconnectPolicy.CanReconnect) { Tracing.For("Protocol").Info(this, "HTTP/1.0 closed, {0} pending — reconnecting", PendingRequestCount); StartReconnect(); @@ -433,11 +428,6 @@ private void TryCompleteAfterEof(bool bodyComplete) } } - private void TryDecodeEof(bool bodyComplete) - { - TryCompleteAfterEof(bodyComplete); - } - private void FailOrphanedRequest() { if (_inFlightRequest is not null) @@ -450,47 +440,36 @@ private void FailOrphanedRequest() private void StartReconnect() { - _reconnectBufferedRequest = _inFlightRequest; + var buffered = _inFlightRequest; _inFlightRequest = null; IsReconnecting = true; - _reconnectAttempts = 1; - _ops.OnOutbound(new ConnectTransport(_transportOptions!)); + _reconnectPolicy.Start(buffered!, _transportOptions!); } private void OnConnectionRestored() { IsReconnecting = false; - _reconnectAttempts = 0; _connectionClosed = false; _decoder.Reset(); - if (_reconnectBufferedRequest is { } req) + if (_reconnectPolicy.TakeBuffered() is { } req) { - _reconnectBufferedRequest = null; EncodeRequest(req); } } private void OnReconnectAttemptFailed() { - if (_reconnectAttempts >= _options.Http1.MaxReconnectAttempts) + var attemptsAtFailure = _reconnectPolicy.Attempts; + + if (_reconnectPolicy.OnAttemptFailed(_transportOptions!, out var buffered)) { - Tracing.For("Protocol").Info(this, "HTTP/1.0 reconnect failed after {0} attempts", _reconnectAttempts); - if (_reconnectBufferedRequest is { } buffered) - { - buffered.Fail(new HttpRequestException("HTTP/1.0 reconnect failed after max attempts.")); - _reconnectBufferedRequest = null; - } + Tracing.For("Protocol").Info(this, "HTTP/1.0 reconnect failed after {0} attempts", attemptsAtFailure); + buffered?.Fail(new HttpRequestException("HTTP/1.0 reconnect failed after max attempts.")); IsReconnecting = false; - _reconnectAttempts = 0; _connectionDead = true; - _ops.OnOutbound(new DisconnectTransport(DisconnectReason.Error)); - return; } - - _reconnectAttempts++; - _ops.OnOutbound(new ConnectTransport(_transportOptions!)); } private void CompleteResponse(HttpResponseMessage response) diff --git a/src/GaudiHTTP/Protocol/Syntax/Http10/Server/Http10ServerStateMachine.cs b/src/GaudiHTTP/Protocol/Syntax/Http10/Server/Http10ServerStateMachine.cs index 3e42749e..a99c5f73 100644 --- a/src/GaudiHTTP/Protocol/Syntax/Http10/Server/Http10ServerStateMachine.cs +++ b/src/GaudiHTTP/Protocol/Syntax/Http10/Server/Http10ServerStateMachine.cs @@ -13,20 +13,12 @@ namespace GaudiHTTP.Protocol.Syntax.Http10.Server; internal sealed class Http10ServerStateMachine : IServerStateMachine, IBodyDrainTarget { - private const string DataRateCheck = "data-rate-check"; - private readonly IServerStageOperations _ops; private readonly Http10ServerDecoder _decoder; private readonly Http10ServerEncoder _encoder; private readonly long _maxRequestBodySize; private readonly int _responseBodyChunkSize; - private readonly DataRateMonitor _requestRate; - private readonly DataRateMonitor _responseRate; - private readonly List _rateViolations = []; - private bool _rateTimerActive; - private readonly TimeProvider _clock; - - private long Now() => _clock.GetUtcNow().ToUnixTimeMilliseconds(); + private readonly ConnectionRateGuard _rateGuard; private IFeatureCollection? _deferredFeatures; private bool _bodyStreaming; @@ -51,11 +43,7 @@ public Http10ServerStateMachine(Http1ConnectionOptions options, IServerStageOper ArgumentNullException.ThrowIfNull(options); _maxRequestBodySize = options.Limits.MaxRequestBodySize; _responseBodyChunkSize = options.ResponseBodyChunkSize; - _clock = timeProvider ?? TimeProvider.System; - - var rate = options.ToRateMonitor(); - _requestRate = new DataRateMonitor(rate.MinRequestBodyDataRate, rate.MinRequestBodyDataRateGracePeriod); - _responseRate = new DataRateMonitor(rate.MinResponseDataRate, rate.MinResponseDataRateGracePeriod); + _rateGuard = new ConnectionRateGuard(ops, options.ToRateMonitor(), timeProvider); _decoder = new Http10ServerDecoder(options.ToHttp10DecoderOptions()); _encoder = new Http10ServerEncoder(options.ToHttp10EncoderOptions()); @@ -76,8 +64,7 @@ void IBodyDrainTarget.EmitDataFrames(int streamId, ReadOnlyMemory data, bo { if (!data.IsEmpty) { - _responseRate.Observe(0, data.Length, Now()); - EnsureRateTimer(); + _rateGuard.ObserveResponse(0, data.Length); var item = TransportBuffer.Rent(data.Length); data.CopyTo(item.FullMemory); item.Length = data.Length; @@ -94,8 +81,7 @@ void IBodyDrainTarget.EmitOwnedDataFrames(int streamId, IMemoryOwner owner { if (bytesWritten > 0) { - _responseRate.Observe(0, bytesWritten, Now()); - EnsureRateTimer(); + _rateGuard.ObserveResponse(0, bytesWritten); _ops.OnOutbound(TransportData.Rent(TransportBuffer.Wrap(owner, bytesWritten))); Tracing.For("Protocol").Trace(this, "HTTP/1.0 response body chunk flushed (bytes={0})", bytesWritten); _serialPump!.OnCapacityAvailable(); @@ -112,7 +98,7 @@ private void EmitEndStreamIfNeeded(bool endStream) { if (endStream) { - _responseRate.Remove(0); + _rateGuard.RemoveResponse(0); if (_deferredFeatures is not null) { _ops.OnResponseBodyComplete(_deferredFeatures); @@ -135,7 +121,7 @@ void IBodyDrainTarget.OnDrainComplete(int streamId) void IBodyDrainTarget.OnDrainFailed(int streamId, Exception reason) { - _responseRate.Remove(0); + _rateGuard.RemoveResponse(0); if (_deferredFeatures is not null) { _ops.OnResponseBodyComplete(_deferredFeatures); @@ -167,15 +153,14 @@ public void DecodeClientData(ITransportInbound data) var outcome = _decoder.Feed(buffer.Memory[pos..], out _); if (_decoder.LastBodyBytesConsumed > 0) { - _requestRate.Observe(0, _decoder.LastBodyBytesConsumed, Now()); - EnsureRateTimer(); + _rateGuard.ObserveRequest(0, _decoder.LastBodyBytesConsumed); } if (outcome == DecodeOutcome.Complete) { _bodyStreaming = false; _activeStreamingReader = null; - _requestRate.Remove(0); + _rateGuard.RemoveRequest(0); } return; @@ -186,8 +171,7 @@ public void DecodeClientData(ITransportInbound data) if (_decoder.LastBodyBytesConsumed > 0) { - _requestRate.Observe(0, _decoder.LastBodyBytesConsumed, Now()); - EnsureRateTimer(); + _rateGuard.ObserveRequest(0, _decoder.LastBodyBytesConsumed); } if (result is DecodeOutcome.Complete or DecodeOutcome.HeadersReady) @@ -200,7 +184,7 @@ public void DecodeClientData(ITransportInbound data) if (result != DecodeOutcome.HeadersReady) { - _requestRate.Remove(0); + _rateGuard.RemoveRequest(0); } _ops.OnRequest(features); @@ -221,15 +205,14 @@ public void DecodeClientData(ITransportInbound data) var bodyOutcome = _decoder.Feed(buffer.Memory[pos..], out _); if (_decoder.LastBodyBytesConsumed > 0) { - _requestRate.Observe(0, _decoder.LastBodyBytesConsumed, Now()); - EnsureRateTimer(); + _rateGuard.ObserveRequest(0, _decoder.LastBodyBytesConsumed); } if (bodyOutcome == DecodeOutcome.Complete) { _bodyStreaming = false; _activeStreamingReader = null; - _requestRate.Remove(0); + _rateGuard.RemoveRequest(0); } } } @@ -293,25 +276,13 @@ public void OnOutboundFlushed() public void OnTimerFired(string name) { - if (name == DataRateCheck) + if (name == ConnectionRateGuard.TimerName) { - _rateTimerActive = false; - _rateViolations.Clear(); - _requestRate.Check(Now(), _rateViolations); - _responseRate.Check(Now(), _rateViolations); - - if (_rateViolations.Count > 0) + if (_rateGuard.OnTimerFired((req, resp) => + Tracing.For("Protocol").Warning(this, + "data rate violation (reqRate={0}, respRate={1})", req, resp))) { - Tracing.For("Protocol").Warning(this, - "data rate violation (reqRate={0}, respRate={1})", - _requestRate.Count, _responseRate.Count); ShouldComplete = true; - return; - } - - if (_requestRate.Count > 0 || _responseRate.Count > 0) - { - EnsureRateTimer(); } } } @@ -327,10 +298,6 @@ public void OnBodyMessage(object msg) case BodyReadFailed failed: _serialPump?.HandleReadFailed(failed.Reason); break; - - case BodyReadContinue: - _serialPump?.HandleBodyReadContinue(); - break; } } @@ -383,8 +350,7 @@ public void Cleanup() _connectionCts?.Cancel(); _connectionCts?.Dispose(); _connectionCts = null; - _ops.OnCancelTimer(DataRateCheck); - _rateTimerActive = false; + _rateGuard.Cleanup(); } private static long? ExtractContentLength(IHttpResponseFeature? responseFeature) @@ -407,15 +373,4 @@ public void Cleanup() return null; } - - private void EnsureRateTimer() - { - if (_rateTimerActive) - { - return; - } - - _rateTimerActive = true; - _ops.OnScheduleTimer(DataRateCheck, TimeSpan.FromSeconds(1)); - } } diff --git a/src/GaudiHTTP/Protocol/Syntax/Http11/Client/Http11ClientStateMachine.cs b/src/GaudiHTTP/Protocol/Syntax/Http11/Client/Http11ClientStateMachine.cs index 03d0d43f..0ce1a641 100644 --- a/src/GaudiHTTP/Protocol/Syntax/Http11/Client/Http11ClientStateMachine.cs +++ b/src/GaudiHTTP/Protocol/Syntax/Http11/Client/Http11ClientStateMachine.cs @@ -5,6 +5,7 @@ using GaudiHTTP.Client; using GaudiHTTP.Internal; using GaudiHTTP.Protocol.Body; +using GaudiHTTP.Protocol.Semantics; using GaudiHTTP.Streams.Stages.Client; using static Servus.Senf; @@ -18,32 +19,70 @@ internal sealed class Http11ClientStateMachine : IClientStateMachine, IBodyDrain private readonly GaudiClientOptions _options; private readonly Queue _inFlightQueue = new(); - private Queue? _reconnectBufferedQueue; + private readonly ReconnectPolicy> _reconnectPolicy; private readonly int _effectivePipelineDepth; - private int _reconnectAttempts; private TransportOptions? _transportOptions; private HttpResponseMessage? _pendingBodyResponse; private bool _outboundBodyPending; - private bool _connectionCloseReceived; private bool _isChunked; private IStreamingBodyReader? _activeStreamingReader; private TransportBuffer? _heldBuffer; private int _heldBufferOffset; private TransportBuffer? _partialResponse; - private bool _draining; - private bool _connectionDead; + private ConnectionState _connectionState; private SerialBodyPump? _serialPump; private CancellationTokenSource? _connectionCts; internal sealed record StreamingSlotFreed; + // Connection lifecycle. Replaces four independently-tracked booleans + // (IsReconnecting, _connectionCloseReceived, _draining, _connectionDead) with one explicit + // state; `Active` is the default (enum default value 0). + // + // Transition table: + // Active -- construction; also OnConnectionRestored() and Cleanup() + // return here (Cleanup only if not currently Reconnecting) + // Active -> CloseAfterResponses CompleteResponse() when the decoder reports + // Connection: close. No new requests are accepted; requests + // already in flight still drain normally. + // Active | CloseAfterResponses + // -> Reconnecting StartReconnect(), from HandleDisconnect() on an + // ungraceful disconnect with in-flight requests and + // reconnect attempts configured. + // Reconnecting -> Active OnConnectionRestored() (TransportConnected), or + // OnUpstreamFinished() abandoning a reconnect in progress + // because the stage itself is shutting down. + // Reconnecting -> Dead OnReconnectAttemptFailed() once MaxReconnectAttempts is + // exhausted. + // + // Findings from the boolean inventory (Task 9): + // - `_draining` was never assigned `true` anywhere in this class — both of its guarded + // branches (in DecodeResponse and CompleteResponse) were unreachable dead code and are + // dropped here; behavior is unchanged since those branches never executed. + // - CloseAfterResponses and Reconnecting could, in principle, both be "true" under the old + // independent-booleans model (nothing cleared _connectionCloseReceived when StartReconnect + // flipped IsReconnecting). In practice this combination is unreachable: Akka actor thread + // confinement processes TransportConnected (which resets to Active) fully before any + // subsequent TransportData is decoded, so CompleteResponse can never run while + // Reconnecting. Collapsing both bits into one field is therefore safe — CompleteResponse + // only promotes Active -> CloseAfterResponses, never overwriting Reconnecting/Dead. + // - Cleanup() never reset IsReconnecting in the original code (only the other three flags). + // Preserved below by leaving the state untouched when it is already Reconnecting. + private enum ConnectionState + { + Active, + CloseAfterResponses, + Reconnecting, + Dead + } + public bool CanAcceptRequest => - _inFlightQueue.Count < _effectivePipelineDepth && !IsReconnecting && !_outboundBodyPending && - !_connectionCloseReceived && !_draining && !_connectionDead; + _inFlightQueue.Count < _effectivePipelineDepth && !_outboundBodyPending && + _connectionState == ConnectionState.Active; public bool HasInFlightRequests => _inFlightQueue.Count > 0; - public bool IsReconnecting { get; private set; } + public bool IsReconnecting => _connectionState == ConnectionState.Reconnecting; public bool ShouldPauseNetwork => _heldBuffer is not null || (_activeStreamingReader?.IsFull ?? false); @@ -53,7 +92,7 @@ internal int PendingRequestCount { if (IsReconnecting) { - return _reconnectBufferedQueue?.Count ?? 0; + return _reconnectPolicy.Buffered?.Count ?? 0; } return _inFlightQueue.Count; @@ -68,6 +107,7 @@ public Http11ClientStateMachine( { _ops = ops; _options = options; + _reconnectPolicy = new ReconnectPolicy>(ops, options.Http1.MaxReconnectAttempts); var decoderOpts = options.ToHttp11DecoderOptions(); var encoderOpts = options.ToHttp11EncoderOptions(); @@ -302,14 +342,13 @@ public void OnUpstreamFinished() if (IsReconnecting) { - if (_reconnectBufferedQueue is { Count: > 0 }) + if (_reconnectPolicy.TakeBuffered() is { Count: > 0 } buffered) { - RequestFault.FailAll(_reconnectBufferedQueue, + RequestFault.FailAll(buffered, new HttpRequestException("HTTP/1.1 transport closed during reconnect.")); } - IsReconnecting = false; - _reconnectAttempts = 0; + _connectionState = ConnectionState.Active; Tracing.For("Protocol").Debug(this, "HTTP/1.1 transport closed during reconnect"); return; } @@ -346,10 +385,6 @@ public void OnBodyMessage(object msg) case BodyReadFailed failed: _serialPump?.HandleReadFailed(failed.Reason); break; - - case BodyReadContinue: - _serialPump?.HandleBodyReadContinue(); - break; } } @@ -369,9 +404,10 @@ public void Cleanup() _heldBuffer = null; _heldBufferOffset = 0; ClearPartial(); - _connectionCloseReceived = false; - _draining = false; - _connectionDead = false; + if (_connectionState != ConnectionState.Reconnecting) + { + _connectionState = ConnectionState.Active; + } _serialPump?.Cleanup(); _serialPump = null; _connectionCts?.Cancel(); @@ -441,11 +477,6 @@ private void DecodeResponse(TransportBuffer buffer, int startOffset = 0) _inFlightQueue.Dequeue(); } - if (_draining && _inFlightQueue.Count == 0) - { - _draining = false; - } - _decoder.Reset(); continue; } @@ -586,7 +617,7 @@ private void HandleDisconnect(TransportDisconnected disconnect) } } - if (HasInFlightRequests && _options.Http1.MaxReconnectAttempts > 0) + if (HasInFlightRequests && _reconnectPolicy.CanReconnect) { Tracing.For("Protocol").Info(this, "HTTP/1.1 closed, {0} pending — reconnecting", PendingRequestCount); StartReconnect(); @@ -642,25 +673,19 @@ private void FailOrphanedRequests() private void StartReconnect() { - _reconnectBufferedQueue = new Queue(_inFlightQueue); + var buffered = new Queue(_inFlightQueue); _inFlightQueue.Clear(); - IsReconnecting = true; - _reconnectAttempts = 1; - _ops.OnOutbound(new ConnectTransport(_transportOptions!)); + _connectionState = ConnectionState.Reconnecting; + _reconnectPolicy.Start(buffered, _transportOptions!); } private void OnConnectionRestored() { - IsReconnecting = false; - _reconnectAttempts = 0; - _connectionCloseReceived = false; + _connectionState = ConnectionState.Active; _decoder.Reset(); - if (_reconnectBufferedQueue is { Count: > 0 }) + if (_reconnectPolicy.TakeBuffered() is { Count: > 0 } queue) { - var queue = _reconnectBufferedQueue; - _reconnectBufferedQueue = null; - while (queue.Count > 0) { var req = queue.Dequeue(); @@ -671,32 +696,26 @@ private void OnConnectionRestored() private void OnReconnectAttemptFailed() { - if (_reconnectAttempts >= _options.Http1.MaxReconnectAttempts) + var attemptsAtFailure = _reconnectPolicy.Attempts; + + if (_reconnectPolicy.OnAttemptFailed(_transportOptions!, out var buffered)) { - Tracing.For("Protocol").Info(this, "HTTP/1.1 reconnect failed after {0} attempts", _reconnectAttempts); - if (_reconnectBufferedQueue is { Count: > 0 }) + Tracing.For("Protocol").Info(this, "HTTP/1.1 reconnect failed after {0} attempts", attemptsAtFailure); + if (buffered is { Count: > 0 }) { - RequestFault.FailAll(_reconnectBufferedQueue, + RequestFault.FailAll(buffered, new HttpRequestException("HTTP/1.1 reconnect failed after max attempts.")); - _reconnectBufferedQueue.Clear(); } - IsReconnecting = false; - _reconnectAttempts = 0; - _connectionDead = true; - _ops.OnOutbound(new DisconnectTransport(DisconnectReason.Error)); - return; + _connectionState = ConnectionState.Dead; } - - _reconnectAttempts++; - _ops.OnOutbound(new ConnectTransport(_transportOptions!)); } private void CompleteResponse(HttpResponseMessage response) { - if (_decoder.ConnectionWillClose) + if (_decoder.ConnectionWillClose && _connectionState == ConnectionState.Active) { - _connectionCloseReceived = true; + _connectionState = ConnectionState.CloseAfterResponses; } HttpRequestMessage? request = null; @@ -705,11 +724,6 @@ private void CompleteResponse(HttpResponseMessage response) request = _inFlightQueue.Dequeue(); } - if (_draining && _inFlightQueue.Count == 0) - { - _draining = false; - } - if (request is not null) { response.RequestMessage = request; diff --git a/src/GaudiHTTP/Protocol/Syntax/Http11/Server/Http11ServerStateMachine.cs b/src/GaudiHTTP/Protocol/Syntax/Http11/Server/Http11ServerStateMachine.cs index e4bd31ba..dd708a6d 100644 --- a/src/GaudiHTTP/Protocol/Syntax/Http11/Server/Http11ServerStateMachine.cs +++ b/src/GaudiHTTP/Protocol/Syntax/Http11/Server/Http11ServerStateMachine.cs @@ -22,8 +22,6 @@ internal sealed class Http11ServerStateMachine : IServerStateMachine, IBodyDrain private const string RequestHeadersTimer = "request-headers"; private const string BodyConsumptionTimer = "body-consumption"; private const string BodyReadTimer = "body-read"; - private const string DataRateCheck = "data-rate-check"; - private readonly IServerStageOperations _ops; private readonly Http11ServerDecoder _decoder; private readonly Http11ServerEncoder _encoder; @@ -37,13 +35,7 @@ internal sealed class Http11ServerStateMachine : IServerStateMachine, IBodyDrain private readonly Http2ConnectionOptions _h2UpgradeOptions; private readonly bool _allowH2cUpgrade; - private readonly DataRateMonitor _requestRate; - private readonly DataRateMonitor _responseRate; - private readonly List _rateViolations = []; - private bool _rateTimerActive; - private readonly TimeProvider _clock; - - private long Now() => _clock.GetUtcNow().ToUnixTimeMilliseconds(); + private readonly ConnectionRateGuard _rateGuard; private int _pendingResponseCount; private bool _outboundBodyPending; @@ -80,11 +72,7 @@ public Http11ServerStateMachine(Http1ConnectionOptions options, Http2ConnectionO _bodyReadTimeout = options.BodyReadTimeout; _bodyEncoderOptions = options.ToBodyEncoderOptions(); _maxRequestBodySize = options.Limits.MaxRequestBodySize; - _clock = timeProvider ?? TimeProvider.System; - - var rate = options.ToRateMonitor(); - _requestRate = new DataRateMonitor(rate.MinRequestBodyDataRate, rate.MinRequestBodyDataRateGracePeriod); - _responseRate = new DataRateMonitor(rate.MinResponseDataRate, rate.MinResponseDataRateGracePeriod); + _rateGuard = new ConnectionRateGuard(ops, options.ToRateMonitor(), timeProvider); var decOpts = options.ToHttp11DecoderOptions(); var encOpts = options.ToHttp11EncoderOptions(); @@ -122,8 +110,7 @@ void IBodyDrainTarget.EmitDataFrames(int streamId, ReadOnlyMemory data, bo var buf = TransportBuffer.Rent(framedSize); ChunkedFramingHelper.WriteChunk(data.Span, buf.FullMemory.Span); buf.Length = framedSize; - _responseRate.Observe(0, framedSize, Now()); - EnsureRateTimer(); + _rateGuard.ObserveResponse(0, framedSize); _ops.OnOutbound(TransportData.Rent(buf)); } else @@ -131,8 +118,7 @@ void IBodyDrainTarget.EmitDataFrames(int streamId, ReadOnlyMemory data, bo var buf = TransportBuffer.Rent(data.Length); data.CopyTo(buf.FullMemory); buf.Length = data.Length; - _responseRate.Observe(0, data.Length, Now()); - EnsureRateTimer(); + _rateGuard.ObserveResponse(0, data.Length); _ops.OnOutbound(TransportData.Rent(buf)); } @@ -157,15 +143,13 @@ void IBodyDrainTarget.EmitOwnedDataFrames(int streamId, IMemoryOwner owner var buf = TransportBuffer.Rent(framedSize); ChunkedFramingHelper.WriteChunk(owner.Memory.Span[..bytesWritten], buf.FullMemory.Span); buf.Length = framedSize; - _responseRate.Observe(0, framedSize, Now()); - EnsureRateTimer(); + _rateGuard.ObserveResponse(0, framedSize); _ops.OnOutbound(TransportData.Rent(buf)); owner.Dispose(); } else { - _responseRate.Observe(0, bytesWritten, Now()); - EnsureRateTimer(); + _rateGuard.ObserveResponse(0, bytesWritten); _ops.OnOutbound(TransportData.Rent(TransportBuffer.Wrap(owner, bytesWritten))); } @@ -193,19 +177,10 @@ private void EmitEndStreamIfNeeded(bool endStream) EmitChunkedTerminator(_activeResponseFeatures); } - _outboundBodyPending = false; - _responseRate.Remove(0); - if (_activeResponseFeatures is not null) - { - _ops.OnResponseBodyComplete(_activeResponseFeatures); - _activeResponseFeatures = null; - } - + var completedFeatures = _activeResponseFeatures; + _activeResponseFeatures = null; Tracing.For("Protocol").Debug(this, "response body complete"); - if (!ShouldComplete && _keepAliveTimeout > TimeSpan.Zero && _pendingResponseCount == 0) - { - _ops.OnScheduleTimer(KeepAliveTimer, _keepAliveTimeout); - } + CompleteResponse(completedFeatures); } } @@ -216,13 +191,12 @@ void IBodyDrainTarget.OnDrainComplete(int streamId) void IBodyDrainTarget.OnDrainFailed(int streamId, Exception reason) { - _outboundBodyPending = false; - _responseRate.Remove(0); - if (_activeResponseFeatures is not null) - { - _ops.OnResponseBodyComplete(_activeResponseFeatures); - _activeResponseFeatures = null; - } + // Does not route through CompleteResponse: a mid-stream drain failure never sets + // ShouldComplete here (no other caller does either on this path), so calling the full + // epilogue would newly rearm the keep-alive timer on a connection whose response framing + // was left inconsistent. Reset the response bookkeeping only, preserving that behavior. + ResetResponseState(_activeResponseFeatures); + _activeResponseFeatures = null; Tracing.For("Protocol").Warning(this, "response body failed: {0}", reason.Message); } @@ -248,31 +222,19 @@ public void DecodeClientData(ITransportInbound data) { var drained = drainingDecoder.Drain(span[pos..]); pos += drained; - _requestRate.Observe(0, drained, Now()); - EnsureRateTimer(); + _rateGuard.ObserveRequest(0, drained); if (drainingDecoder.IsComplete) { _draining = false; _ops.OnCancelTimer(BodyConsumptionTimer); - _requestRate.Remove(0); + _rateGuard.RemoveRequest(0); _decoder.Reset(); } } else if (_bodyStreaming && _decoder.StreamingReader is not null) { - var outcome = _decoder.Feed(buffer.Memory[pos..], out var bodyConsumed); - pos += bodyConsumed; - _requestRate.Observe(0, bodyConsumed, Now()); - EnsureRateTimer(); - - if (outcome == DecodeOutcome.Complete) - { - _bodyStreaming = false; - _activeStreamingReader = null; - _requestRate.Remove(0); - _decoder.Reset(); - } + ResumeStreamingBody(buffer.Memory, ref pos); } if (!_requestHeadersTimerActive && _pendingResponseCount == 0 && !_bodyStreaming @@ -313,35 +275,11 @@ public void DecodeClientData(ITransportInbound data) ShouldComplete = true; } - var hasBody = outcome == DecodeOutcome.HeadersReady || _decoder.CurrentBodyReader is not null; - var features = FeatureCollectionFactory.Create(hasBody, - out var feature, _ops.ConnectionFeature, - _ops.TlsHandshakeFeature, _maxRequestBodySize); - _decoder.PopulateRequestFeature(feature); - features.Set(new GaudiInformationalResponseFeature((statusCode, headers) => - SendInformational(statusCode, headers))); - - if (!ShouldComplete && feature.Protocol == WellKnownHeaders.Http10) - { - ShouldComplete = true; - } - - if (_allowH2cUpgrade && TryHandleH2cUpgrade(features)) + if (!ProcessDecodedRequest(outcome)) { - _decoder.Reset(); break; } - _pendingResponseCount++; - Tracing.For("Protocol").Debug(this, "request dispatched (pending={0})", _pendingResponseCount); - _ops.OnRequest(features); - - if (string.Equals(feature.Headers[WellKnownHeaders.Expect], "100-continue", - StringComparison.OrdinalIgnoreCase)) - { - SendInformational(100, new HeaderDictionary()); - } - if (outcome == DecodeOutcome.HeadersReady) { _bodyStreaming = true; @@ -354,21 +292,9 @@ public void DecodeClientData(ITransportInbound data) _ops.StageActor.Tell(new BodyResumed(), ActorRefs.NoSender); } - if (pos < buffer.Memory.Length) + if (pos < buffer.Memory.Length && ResumeStreamingBody(buffer.Memory, ref pos)) { - var bodyOutcome = _decoder.Feed(buffer.Memory[pos..], out var bodyConsumed); - pos += bodyConsumed; - _requestRate.Observe(0, bodyConsumed, Now()); - EnsureRateTimer(); - - if (bodyOutcome == DecodeOutcome.Complete) - { - _bodyStreaming = false; - _activeStreamingReader = null; - _requestRate.Remove(0); - _decoder.Reset(); - continue; - } + continue; } break; @@ -390,6 +316,74 @@ public void DecodeClientData(ITransportInbound data) } } + /// + /// Feeds buffered request bytes to the decoder while a request body is being streamed to the + /// handler, advancing by however much was consumed. Used both by the + /// standalone streaming-resume preamble and by the inline body-feed right after headers are + /// parsed — both reset the same streaming state once the decoder reports completion. + /// + /// if the body finished decoding on this feed. + private bool ResumeStreamingBody(ReadOnlyMemory buffer, ref int pos) + { + var outcome = _decoder.Feed(buffer[pos..], out var bodyConsumed); + pos += bodyConsumed; + _rateGuard.ObserveRequest(0, bodyConsumed); + + if (outcome != DecodeOutcome.Complete) + { + return false; + } + + _bodyStreaming = false; + _activeStreamingReader = null; + _rateGuard.RemoveRequest(0); + _decoder.Reset(); + return true; + } + + /// + /// Dispatches a fully- or headers-decoded request: builds the request feature collection, + /// applies the HTTP/1.0 keep-alive rule and the h2c Upgrade handshake, hands the request to + /// the bridge, and triggers an Expect: 100-continue informational response if requested. + /// + /// + /// if the request instead triggered an h2c protocol switch, signaling + /// the caller to stop parsing further requests off this connection. + /// + private bool ProcessDecodedRequest(DecodeOutcome outcome) + { + var hasBody = outcome == DecodeOutcome.HeadersReady || _decoder.CurrentBodyReader is not null; + var features = FeatureCollectionFactory.Create(hasBody, + out var feature, _ops.ConnectionFeature, + _ops.TlsHandshakeFeature, _maxRequestBodySize); + _decoder.PopulateRequestFeature(feature); + features.Set(new GaudiInformationalResponseFeature((statusCode, headers) => + SendInformational(statusCode, headers))); + + if (!ShouldComplete && feature.Protocol == WellKnownHeaders.Http10) + { + ShouldComplete = true; + } + + if (_allowH2cUpgrade && TryHandleH2cUpgrade(features)) + { + _decoder.Reset(); + return false; + } + + _pendingResponseCount++; + Tracing.For("Protocol").Debug(this, "request dispatched (pending={0})", _pendingResponseCount); + _ops.OnRequest(features); + + if (string.Equals(feature.Headers[WellKnownHeaders.Expect], "100-continue", + StringComparison.OrdinalIgnoreCase)) + { + SendInformational(100, new HeaderDictionary()); + } + + return true; + } + private void ReconcileBodyReadTimer() { if (_bodyStreaming && _bodyReadTimeout > TimeSpan.Zero) @@ -469,50 +463,21 @@ public void OnResponse(IFeatureCollection features) { // Headers-only response (1xx/204/304 or HEAD): no body drain will run, so recycle the // feature collection now. Safe — the SM keeps no reference to `features` on this path. - _ops.OnResponseBodyComplete(features); - - if (!ShouldComplete && _keepAliveTimeout > TimeSpan.Zero && _pendingResponseCount == 0) - { - _ops.OnScheduleTimer(KeepAliveTimer, _keepAliveTimeout); - } + CompleteResponse(features); return; } - if (_decoder.CurrentBodyReader is { IsCompleted: false }) - { - if (_bodyStreaming) - { - _bodyStreaming = false; - _activeStreamingReader = null; - if (_bodyReadTimerActive) - { - _ops.OnCancelTimer(BodyReadTimer); - _bodyReadTimerActive = false; - } - } - - _draining = true; - Tracing.For("Protocol").Debug(this, "draining unconsumed request body"); - - if (_bodyConsumptionTimeout > TimeSpan.Zero) - { - _ops.OnScheduleTimer(BodyConsumptionTimer, _bodyConsumptionTimeout); - } - } + ScheduleRequestBodyDrainIfUnconsumed(); if (gaudiBody is not null) { if (coalesceBody) { // Body bytes were folded into the header buffer above: nothing more to emit. - _ops.OnResponseBodyComplete(features); Tracing.For("Protocol").Debug(this, "response body complete (buffered, coalesced, bytes={0})", bufferedBody.Length); - if (!ShouldComplete && _keepAliveTimeout > TimeSpan.Zero && _pendingResponseCount == 0) - { - _ops.OnScheduleTimer(KeepAliveTimer, _keepAliveTimeout); - } + CompleteResponse(features); return; } @@ -537,13 +502,72 @@ public void OnResponse(IFeatureCollection features) else { // No streamed body feature to drain: recycle the feature collection now. - _ops.OnResponseBodyComplete(features); + CompleteResponse(features); + } + } + + /// + /// A response is being sent while the previous request's body is still unconsumed by the + /// handler. HTTP/1.1 pipelining matches responses to requests by wire position (RFC 9112 + /// §9.3.2), so the leftover request bytes must be drained off the wire before the next + /// request can be parsed — otherwise they would be misread as the start of the next request. + /// + private void ScheduleRequestBodyDrainIfUnconsumed() + { + if (_decoder.CurrentBodyReader is not { IsCompleted: false }) + { + return; + } - if (!ShouldComplete && _keepAliveTimeout > TimeSpan.Zero && _pendingResponseCount == 0) + if (_bodyStreaming) + { + _bodyStreaming = false; + _activeStreamingReader = null; + if (_bodyReadTimerActive) { - _ops.OnScheduleTimer(KeepAliveTimer, _keepAliveTimeout); + _ops.OnCancelTimer(BodyReadTimer); + _bodyReadTimerActive = false; } } + + _draining = true; + Tracing.For("Protocol").Debug(this, "draining unconsumed request body"); + + if (_bodyConsumptionTimeout > TimeSpan.Zero) + { + _ops.OnScheduleTimer(BodyConsumptionTimer, _bodyConsumptionTimeout); + } + } + + /// + /// Shared tail for every response-completion path that legitimately reaches the end of a + /// response (as opposed to , which tears the + /// connection down instead): reset per-response bookkeeping, recycle the feature collection, + /// and rearm the keep-alive timer if the connection is otherwise idle. + /// + private void CompleteResponse(IFeatureCollection? features) + { + ResetResponseState(features); + + if (!ShouldComplete && _keepAliveTimeout > TimeSpan.Zero && _pendingResponseCount == 0) + { + _ops.OnScheduleTimer(KeepAliveTimer, _keepAliveTimeout); + } + } + + /// + /// Clears outbound-body-pending state, drops the response rate-monitor entry (a no-op if the + /// path never observed it — otherwise an idle keep-alive connection would be flagged as a + /// stalled response once the grace period elapses), and recycles the feature collection. + /// + private void ResetResponseState(IFeatureCollection? features) + { + _outboundBodyPending = false; + _rateGuard.RemoveResponse(0); + if (features is not null) + { + _ops.OnResponseBodyComplete(features); + } } private void EmitBufferedBody(IFeatureCollection features, ReadOnlyMemory body, bool isChunked) @@ -562,8 +586,7 @@ private void EmitBufferedBody(IFeatureCollection features, ReadOnlyMemory var buf = TransportBuffer.Rent(framedSize); ChunkedFramingHelper.WriteChunk(chunk.Span, buf.FullMemory.Span); buf.Length = framedSize; - _responseRate.Observe(0, framedSize, Now()); - EnsureRateTimer(); + _rateGuard.ObserveResponse(0, framedSize); _ops.OnOutbound(TransportData.Rent(buf)); } else @@ -571,8 +594,7 @@ private void EmitBufferedBody(IFeatureCollection features, ReadOnlyMemory var buf = TransportBuffer.Rent(take); chunk.CopyTo(buf.FullMemory); buf.Length = take; - _responseRate.Observe(0, take, Now()); - EnsureRateTimer(); + _rateGuard.ObserveResponse(0, take); _ops.OnOutbound(TransportData.Rent(buf)); } @@ -585,16 +607,8 @@ private void EmitBufferedBody(IFeatureCollection features, ReadOnlyMemory EmitChunkedTerminator(features); } - // The response is fully handed to the transport: drop the rate entry, or the idle - // keep-alive connection is flagged as a violation once the grace period elapses. - _responseRate.Remove(0); - _ops.OnResponseBodyComplete(features); - Tracing.For("Protocol").Debug(this, "response body complete (buffered, bytes={0})", body.Length); - if (!ShouldComplete && _keepAliveTimeout > TimeSpan.Zero && _pendingResponseCount == 0) - { - _ops.OnScheduleTimer(KeepAliveTimer, _keepAliveTimeout); - } + CompleteResponse(features); } private void EmitChunkedTerminator(IFeatureCollection? features) @@ -690,25 +704,14 @@ public void OnTimerFired(string name) _bodyReadTimerActive = false; ShouldComplete = true; } - else if (name == DataRateCheck) + else if (name == ConnectionRateGuard.TimerName) { - _rateTimerActive = false; - _rateViolations.Clear(); - _requestRate.Check(Now(), _rateViolations); - _responseRate.Check(Now(), _rateViolations); - - if (_rateViolations.Count > 0) + if (_rateGuard.OnTimerFired((req, resp) => + Tracing.For("Protocol").Warning(this, + "data rate violation (reqRate={0}, respRate={1}, paused={2})", + req, resp, ShouldPauseNetwork))) { - Tracing.For("Protocol").Warning(this, - "data rate violation (reqRate={0}, respRate={1}, paused={2})", - _requestRate.Count, _responseRate.Count, ShouldPauseNetwork); ShouldComplete = true; - return; - } - - if (_requestRate.Count > 0 || _responseRate.Count > 0) - { - EnsureRateTimer(); } } } @@ -724,10 +727,6 @@ public void OnBodyMessage(object msg) case BodyReadFailed failed: _serialPump?.HandleReadFailed(failed.Reason); break; - - case BodyReadContinue: - _serialPump?.HandleBodyReadContinue(); - break; } } @@ -862,18 +861,6 @@ public void Cleanup() _ops.OnCancelTimer(KeepAliveTimer); _ops.OnCancelTimer(BodyConsumptionTimer); - _ops.OnCancelTimer(DataRateCheck); - _rateTimerActive = false; - } - - private void EnsureRateTimer() - { - if (_rateTimerActive) - { - return; - } - - _rateTimerActive = true; - _ops.OnScheduleTimer(DataRateCheck, TimeSpan.FromSeconds(1)); + _rateGuard.Cleanup(); } } \ No newline at end of file diff --git a/src/GaudiHTTP/Protocol/Syntax/Http2/Client/Http2ClientSessionManager.cs b/src/GaudiHTTP/Protocol/Syntax/Http2/Client/Http2ClientSessionManager.cs index a3f7f687..833a5231 100644 --- a/src/GaudiHTTP/Protocol/Syntax/Http2/Client/Http2ClientSessionManager.cs +++ b/src/GaudiHTTP/Protocol/Syntax/Http2/Client/Http2ClientSessionManager.cs @@ -31,7 +31,7 @@ internal sealed record AbandonedResponseBody(int StreamId); private readonly Dictionary _streams = new(); private readonly Dictionary _drainContentOwners = new(); private readonly CancellationTokenSource _connectionCts = new(); - private FlowControlledBodyPump? _scheduler; + private FlowControlledBodyPump? _pump; private readonly int _maxBufferedResponseBodySize; private bool _prefaceSent; @@ -92,7 +92,7 @@ public Http2ClientSessionManager( _responseDecoder.SetMaxAllowedTableSize(_encoderOptions.HeaderTableSize); // RFC 9113 §4.2: enforce the MAX_FRAME_SIZE we advertise in the preface on inbound frames. _frameDecoder = new FrameDecoder(_encoderOptions.MaxFrameSize); - _scheduler = new FlowControlledBodyPump(this, _flow, _connectionCts, _requestEncoder.MaxFrameSize, 256); + _pump = new FlowControlledBodyPump(this, _flow, _connectionCts, _requestEncoder.MaxFrameSize, 256); } public TransportData? TryBuildPreface() @@ -135,22 +135,7 @@ public void EncodeRequest(HttpRequestMessage request) return; } - var endpoint = request.RequestUri is not null - ? RequestEndpoint.FromRequest(request) - : RequestEndpoint.Default; - - if (Endpoint == default && endpoint != default) - { - Endpoint = endpoint; - var transportOptions = OptionsFactory.Build(Endpoint, _options); - _ops.OnOutbound(new ConnectTransport(transportOptions)); - - var preface = TryBuildPreface(); - if (preface is not null) - { - _ops.OnOutbound(preface); - } - } + EnsureConnected(request); _correlationMap.TryAdd(streamId, request); @@ -200,6 +185,45 @@ public void EncodeRequest(HttpRequestMessage request) _streams[streamId] = state; } + SendRequestBody(streamId, state, request); + } + + /// + /// Lazily bootstraps the connection to the request's endpoint on the first request that + /// carries one: opens the transport and, if the preface hasn't been sent yet, sends it + /// immediately after. A no-op once has already been set for this + /// connection. + /// + private void EnsureConnected(HttpRequestMessage request) + { + var endpoint = request.RequestUri is not null + ? RequestEndpoint.FromRequest(request) + : RequestEndpoint.Default; + + if (Endpoint != default || endpoint == default) + { + return; + } + + Endpoint = endpoint; + var transportOptions = OptionsFactory.Build(Endpoint, _options); + _ops.OnOutbound(new ConnectTransport(transportOptions)); + + var preface = TryBuildPreface(); + if (preface is not null) + { + _ops.OnOutbound(preface); + } + } + + /// + /// Sends the request body via the fastest strategy the content supports, in order: an inline + /// slice of an already-materialized MemoryStream buffer, inline synchronous serialization + /// into a pooled array, a direct empty-body END_STREAM (no scheduler involved), or - the + /// general fallback - registering with the body-drain scheduler for async chunked reads. + /// + private void SendRequestBody(int streamId, StreamState state, HttpRequestMessage request) + { var contentLength = request.Content?.Headers.ContentLength; var bodyStream = request.Content?.ReadAsStream(); @@ -252,7 +276,7 @@ public void EncodeRequest(HttpRequestMessage request) state.MarkBodyDrainActive(); _drainContentOwners[streamId] = request.Content!; - _scheduler!.Register(streamId, bodyStream!, request.Content?.Headers.ContentLength, request.GetCancellationToken()); + _pump!.Register(streamId, bodyStream!, request.Content?.Headers.ContentLength, request.GetCancellationToken()); } private void EmitBodyDirect(int streamId, StreamState state, Memory body) @@ -276,11 +300,14 @@ private void EmitBodyDirect(int streamId, StreamState state, Memory body) // All data sent inline - mark complete and release stream state. CloseStream(streamId); - if (state.IsRemoteClosed) + // HasBodyDrain is always false here (this branch never activated a drain), so + // MayRelease reduces to state.IsRemoteClosed - i.e. release only if the response + // already fully arrived (it cannot have, since this all runs synchronously inside + // EncodeRequest before any inbound frame is processed; kept as a real gate rather + // than an assumption so a future reentrant caller stays correct). + if (state.MayRelease) { - _streams.Remove(streamId); - ReturnBodyReader(state); - state.Dispose(); + ReleaseStream(streamId, state); } return; @@ -289,7 +316,7 @@ private void EmitBodyDirect(int streamId, StreamState state, Memory body) // Window exhausted before all data sent: hand the remainder to the scheduler // which will emit it when the send window opens up via WINDOW_UPDATE. state.MarkBodyDrainActive(); - _scheduler!.Register(streamId, new MemoryStream(body[sent..].ToArray(), writable: false), null, CancellationToken.None); + _pump!.Register(streamId, new MemoryStream(body[sent..].ToArray(), writable: false), null, CancellationToken.None); } private bool TrySerializeBodyDirect(HttpContent content, int streamId, StreamState state, int bodyLength) @@ -489,7 +516,7 @@ public void Cleanup() state.AbortBody(); } - _scheduler?.Cleanup(); + _pump?.Cleanup(); _drainContentOwners.Clear(); ReleaseAllStreamState(); } @@ -555,18 +582,17 @@ void IBodyDrainTarget.OnDrainComplete(int streamId) { _drainContentOwners.Remove(streamId); + var releasable = false; if (_streams.TryGetValue(streamId, out var state)) { - state.MarkBodyDrainComplete(); + releasable = state.OnLocalDone(); } CloseStream(streamId); - if (state is { IsRemoteClosed: true }) + if (releasable) { - _streams.Remove(streamId); - ReturnBodyReader(state); - state.Dispose(); + ReleaseStream(streamId, state!); } } @@ -703,6 +729,15 @@ private void HandleRstStream(RstStreamFrame rst) CloseStream(rst.StreamId); } + // Deliberately asymmetric with the server's CloseStream (which does remove from _streams): + // TryCancelStream calls CloseStream but never releases the StreamState itself (no + // ReleaseStream call there), relying on it staying in _streams so a later inbound frame for + // the same stream (HEADERS still in flight when the caller cancelled, a trailing DATA/RST, + // etc.) finds the SAME state object - with its already-accumulated header buffer and body + // reader - and drives it through the normal teardown sites instead of silently renting a + // fresh, uncorrelated StreamState. Removing here would also make CloseStream's caller in + // OnDrainComplete/ProcessDataFrame race the ReleaseStream calls those methods still need to + // make explicitly, since CloseStream has no reference to release the body reader itself. private void CloseStream(int streamId) { if (_streams.TryGetValue(streamId, out var state) && state.HasBodyReader) @@ -710,7 +745,7 @@ private void CloseStream(int streamId) state.AbortBody(); } - _scheduler?.Cancel(streamId); + _pump?.Cancel(streamId); _tracker.OnStreamClosed(streamId); _flow.RemoveStreamSendWindow(streamId); @@ -721,6 +756,20 @@ private void CloseStream(int streamId) } } + /// + /// Single teardown point for a fully-closed (RFC 9113 §5.1) stream: removes it from + /// , returns its body reader, and disposes it back to the pool. Callers + /// decide when a stream is releasable (see , + /// , ) - this method + /// only performs the mechanical release once that decision has been made. + /// + private void ReleaseStream(int streamId, StreamState state) + { + _streams.Remove(streamId); + ReturnBodyReader(state); + state.Dispose(); + } + private void HandleHeaders(HeadersFrame frame) { if (!_streams.TryGetValue(frame.StreamId, out var state)) @@ -782,13 +831,9 @@ private void HandleData(DataFrame frame) DispatchBufferedResponse(frame.StreamId, state, buffered); } - state.MarkRemoteClosed(); - - if (!state.HasBodyDrain || state.IsBodyDrainComplete) + if (state.OnRemoteDone()) { - _streams.Remove(frame.StreamId); - ReturnBodyReader(state); - state.Dispose(); + ReleaseStream(frame.StreamId, state); } } } @@ -841,9 +886,11 @@ private void DecodeHeaders(int streamId, bool endStream) DispatchBufferedResponse(streamId, state, buffered); } - _streams.Remove(streamId); - ReturnBodyReader(state); - state.Dispose(); + // Trailers carry END_STREAM directly (no MarkRemoteClosed/OnRemoteDone call + // needed) - reaching here IS the terminal remote-closed event, so release + // unconditionally rather than gating on IsLocalSendComplete like the + // HandleData/OnDrainComplete sites do (matches original behavior). + ReleaseStream(streamId, state); } return; @@ -870,9 +917,9 @@ private void DecodeHeaders(int streamId, bool endStream) if (endStream) { _correlationMap.Remove(streamId); - _streams.Remove(streamId); - ReturnBodyReader(state); - state.Dispose(); + // Interim (1xx) response terminating the stream is itself the terminal + // remote-closed event; release unconditionally (matches original behavior). + ReleaseStream(streamId, state); } return; @@ -891,9 +938,10 @@ private void DecodeHeaders(int streamId, bool endStream) _ops.OnResponse(response); - _streams.Remove(streamId); - ReturnBodyReader(state); - state.Dispose(); + // A final response whose own HEADERS carried END_STREAM has no body at all - this IS + // the terminal remote-closed event; release unconditionally (matches original + // behavior). + ReleaseStream(streamId, state); return; } @@ -952,16 +1000,12 @@ public void OnBodyMessage(object msg) { switch (msg) { - case BodyReadContinue dc: - _scheduler?.HandleBodyReadContinue(dc.StreamId); - break; - case BodyReadComplete read: - _scheduler?.HandleReadComplete(read.StreamId, read.BytesRead); + _pump?.HandleReadComplete(read.StreamId, read.BytesRead); break; case BodyReadFailed failed: - _scheduler?.HandleReadFailed(failed.StreamId, failed.Reason); + _pump?.HandleReadFailed(failed.StreamId, failed.Reason); break; case AbandonedResponseBody abandoned: @@ -982,9 +1026,9 @@ private void OnResponseBodyAbandoned(int streamId) state.AbortBody(); EmitFrame(new RstStreamFrame(streamId, Http2ErrorCode.NoError)); - _streams.Remove(streamId); - ReturnBodyReader(state); - state.Dispose(); + // Abandonment forces closure regardless of any drain gate - release unconditionally + // (matches original behavior). + ReleaseStream(streamId, state); if (!_tracker.OnStreamClosed(streamId)) { @@ -1002,7 +1046,7 @@ private void OnResponseBodyAbandoned(int streamId) private void HandleWindowUpdate(WindowUpdateFrame frame) { _flow.OnSendWindowUpdate(frame.StreamId, frame.Increment); - _scheduler?.OnWindowUpdate(frame.StreamId); + _pump?.OnWindowUpdate(frame.StreamId); } private void ReturnBodyReader(StreamState state) diff --git a/src/GaudiHTTP/Protocol/Syntax/Http2/Server/Http2ServerSessionManager.cs b/src/GaudiHTTP/Protocol/Syntax/Http2/Server/Http2ServerSessionManager.cs index 5a5ea191..b92513d1 100644 --- a/src/GaudiHTTP/Protocol/Syntax/Http2/Server/Http2ServerSessionManager.cs +++ b/src/GaudiHTTP/Protocol/Syntax/Http2/Server/Http2ServerSessionManager.cs @@ -308,50 +308,7 @@ public void OnResponse(IFeatureCollection features) { if (bufferedBody.Length > 0) { - var window = _flow.GetSendWindow(streamId); - if (window >= bufferedBody.Length) - { - var bufferedFeatures = state.GetFeatures(); - var trailerFeature = bufferedFeatures?.Get(); - var hasTrailers = trailerFeature?.Trailers.Count > 0; - - if (hasTrailers) - { - if (trailerFeature!.Trailers is GaudiHeaderDictionary gaudiTrailers) - { - gaudiTrailers.SetReadOnly(); - } - - var trailerFrames = _responseEncoder.EncodeTrailers(streamId, trailerFeature.Trailers); - if (trailerFrames.Count > 0) - { - EmitBufferedDataFrames(streamId, bufferedBody, endStream: false); - _flow.OnDataSent(streamId, bufferedBody.Length); - for (var i = 0; i < trailerFrames.Count; i++) - { - EmitFrame(trailerFrames[i]); - } - } - else - { - EmitBufferedDataFrames(streamId, bufferedBody, endStream: true); - _flow.OnDataSent(streamId, bufferedBody.Length); - } - } - else - { - EmitBufferedDataFrames(streamId, bufferedBody, endStream: true); - _flow.OnDataSent(streamId, bufferedBody.Length); - } - CloseStream(streamId); - return; - } - - // Window can't take the whole body: emit what fits now and hold the rest as a slice - // (no copy — it points into the still-live response buffer), emitting it directly on - // WINDOW_UPDATE. SendBufferedBodyWithFlowControl handles window == 0 too (emits - // nothing now, holds the whole body). No ToArray, no MemoryStream, no pump. - SendBufferedBodyWithFlowControl(streamId, state, bufferedBody, window); + SendBufferedResponseBody(streamId, state, bufferedBody); } else { @@ -363,7 +320,6 @@ public void OnResponse(IFeatureCollection features) } var bodyStream = gaudiBody.GetResponseStream(); - state.MarkBodyDrainActive(); _pump!.Register(streamId, bodyStream, contentLength, CancellationToken.None); Tracing.For("Protocol").Debug(this, "HTTP/2: response body drain started (stream={0})", streamId); } @@ -391,10 +347,6 @@ public void OnBodyMessage(object msg) { switch (msg) { - case BodyReadContinue dc: - _pump?.HandleBodyReadContinue(dc.StreamId); - break; - case BodyReadComplete read: _pump?.HandleReadComplete(read.StreamId, read.BytesRead); break; @@ -414,7 +366,14 @@ public void OnBodyMessage(object msg) } } - private void EmitEndOfBody(int streamId, StreamState state) + /// + /// Terminates a response body, optionally sending first (the + /// buffered-body path in routes its already-materialized + /// body through here instead of duplicating the trailer-vs-no-trailer framing). With no + /// preceding data (the pumped/deferred-body paths), only an END_STREAM-carrying frame is sent - + /// an empty DATA frame, or trailers preceded by an empty non-terminal DATA frame. + /// + private void EmitEndOfBody(int streamId, StreamState state, ReadOnlyMemory precedingData = default) { var features = state.GetFeatures(); var trailerFeature = features?.Get(); @@ -430,7 +389,16 @@ private void EmitEndOfBody(int streamId, StreamState state) var trailerFrames = _responseEncoder.EncodeTrailers(streamId, trailerFeature.Trailers); if (trailerFrames.Count > 0) { - EmitFrame(new DataFrame(streamId, ReadOnlyMemory.Empty, endStream: false)); + if (precedingData.IsEmpty) + { + EmitFrame(new DataFrame(streamId, ReadOnlyMemory.Empty, endStream: false)); + } + else + { + EmitBufferedDataFrames(streamId, precedingData, endStream: false); + _flow.OnDataSent(streamId, precedingData.Length); + } + for (var i = 0; i < trailerFrames.Count; i++) { EmitFrame(trailerFrames[i]); @@ -438,13 +406,25 @@ private void EmitEndOfBody(int streamId, StreamState state) } else { - EmitFrame(new DataFrame(streamId, ReadOnlyMemory.Empty, endStream: true)); + EmitBodyThenEndStream(streamId, precedingData); } } else + { + EmitBodyThenEndStream(streamId, precedingData); + } + } + + private void EmitBodyThenEndStream(int streamId, ReadOnlyMemory data) + { + if (data.IsEmpty) { EmitFrame(new DataFrame(streamId, ReadOnlyMemory.Empty, endStream: true)); + return; } + + EmitBufferedDataFrames(streamId, data, endStream: true); + _flow.OnDataSent(streamId, data.Length); } public void SendKeepAlivePing() @@ -639,8 +619,7 @@ private void HandleDataFrame(DataFrame data) if (!data.Data.IsEmpty) { - _requestRate.Observe(streamId, data.Data.Length, Now()); - EnsureRateTimer(); + ObserveRate(_requestRate, streamId, data.Data.Length); } } @@ -948,11 +927,49 @@ private void CloseStream(int streamId) _flow.RemoveStreamSendWindow(streamId); - ReturnBodyReader(state); - state.Dispose(); + // Unlike the client, the server never gates release on an outbound drain (HasBodyDrain + // is write-only on this side - the response-body pump owns its own completion signal + // via IBodyDrainTarget.OnDrainComplete, which itself routes back through CloseStream). + // CloseStream is the single terminal event for both directions here, so release is + // always unconditional - see StreamState.MayRelease for the client's gated equivalent. + ReleaseStream(streamId, state); + } + } + + /// + /// Single teardown point for a stream: removes it from , returns its body + /// reader, and disposes it back to the pool. Mirrors the client session manager's + /// ReleaseStream - kept here as its own method (rather than shared) because the two + /// managers have no common base and the body is only three lines. + /// + private void ReleaseStream(int streamId, StreamState state) + { + ReturnBodyReader(state); + state.Dispose(); + _streams.Remove(streamId); + } - _streams.Remove(streamId); + /// + /// Sends a non-empty buffered response body. If the current send window covers the whole + /// body, routes it through as preceding data (so the trailer-vs-no- + /// trailer framing lives in one place, shared with the pumped/deferred body path). Otherwise + /// emits what fits now and holds the rest as a flow-controlled remainder. + /// + private void SendBufferedResponseBody(int streamId, StreamState state, ReadOnlyMemory bufferedBody) + { + var window = _flow.GetSendWindow(streamId); + if (window >= bufferedBody.Length) + { + EmitEndOfBody(streamId, state, bufferedBody); + CloseStream(streamId); + return; } + + // Window can't take the whole body: emit what fits now and hold the rest as a slice + // (no copy — it points into the still-live response buffer), emitting it directly on + // WINDOW_UPDATE. SendBufferedBodyWithFlowControl handles window == 0 too (emits + // nothing now, holds the whole body). No ToArray, no MemoryStream, no pump. + SendBufferedBodyWithFlowControl(streamId, state, bufferedBody, window); } private void SendBufferedBodyWithFlowControl(int streamId, StreamState state, ReadOnlyMemory body, @@ -980,7 +997,6 @@ private void SendBufferedBodyWithFlowControl(int streamId, StreamState state, Re // arrives. The slice points into the response feature's WrittenMemory, which stays valid // until the stream is closed (after the body fully drains) — see DrainBufferedRemainder. state.SetBufferedRemainder(remainder); - state.MarkBodyDrainActive(); Tracing.For("Protocol").Debug(this, "HTTP/2: buffered body flow-controlled (stream={0}, sent={1}, queued={2})", @@ -1012,7 +1028,6 @@ private void DrainBufferedRemainder(int streamId) if (!state.HasBufferedRemainder) { - state.MarkBodyDrainComplete(); EmitEndOfBody(streamId, state); CloseStream(streamId); } @@ -1067,7 +1082,6 @@ void IBodyDrainTarget.OnDrainComplete(int streamId) if (_streams.TryGetValue(streamId, out var state)) { - state.MarkBodyDrainComplete(); EmitEndOfBody(streamId, state); } @@ -1121,8 +1135,7 @@ private void EmitBufferedDataFrames(int streamId, ReadOnlyMemory body, boo if (rateActive) { - _responseRate.Observe(streamId, body.Length, Now()); - EnsureRateTimer(); + ObserveRate(_responseRate, streamId, body.Length); } buf.Length = offset; @@ -1135,12 +1148,11 @@ private void EmitFrame(Http2Frame frame) { Tracing.For("Protocol").Trace(this, "HTTP/2: DATA out (stream={0}, len={1}, endStream={2})", d.StreamId, d.Data.Length, d.EndStream); - } - if (frame is DataFrame { Data.Length: > 0 } df) - { - _responseRate.Observe(df.StreamId, df.Data.Length, Now()); - EnsureRateTimer(); + if (d.Data.Length > 0) + { + ObserveRate(_responseRate, d.StreamId, d.Data.Length); + } } var totalSize = frame.SerializedSize; @@ -1151,6 +1163,17 @@ private void EmitFrame(Http2Frame frame) _ops.OnOutbound(TransportData.Rent(buf)); } + /// + /// Records a data-rate sample and (re)arms the shared check timer if it isn't already running. + /// Shared by the request-rate and response-rate observation sites (DATA in, buffered DATA out, + /// single-frame DATA out) so the pair never drifts apart. + /// + private void ObserveRate(DataRateMonitor monitor, int streamId, int length) + { + monitor.Observe(streamId, length, Now()); + EnsureRateTimer(); + } + public void EmitRstStream(int streamId, Http2ErrorCode errorCode) { Tracing.For("Protocol").Debug(this, "HTTP/2: RST_STREAM (stream={0}, error={1})", streamId, errorCode); diff --git a/src/GaudiHTTP/Protocol/Syntax/Http2/StreamState.cs b/src/GaudiHTTP/Protocol/Syntax/Http2/StreamState.cs index 3c912bbe..e6761db5 100644 --- a/src/GaudiHTTP/Protocol/Syntax/Http2/StreamState.cs +++ b/src/GaudiHTTP/Protocol/Syntax/Http2/StreamState.cs @@ -57,8 +57,6 @@ public void SetTimerKeys(int streamId) public bool IsBodyDrainComplete { get; private set; } - public bool IsBodyReadPending { get; set; } - /// /// Declared Content-Length of the body being fed, when known. When set, an END_STREAM /// arriving before (or after) exactly this many bytes faults the body reader instead of @@ -83,11 +81,6 @@ public void InitResponse(HttpResponseMessage response) _response = response; } - public HttpResponseMessage GetOrCreateResponse() - { - return _response ??= new HttpResponseMessage(); - } - public HttpResponseMessage GetResponse() { return _response ?? throw new InvalidOperationException("No response has been initialized."); @@ -163,11 +156,6 @@ public void InitBodyReader(IBodyReader reader, long maxBodySize = long.MaxValue) ExpectedBodyLength = PeekContentLength(); } - public void DetachBodyReader() - { - _bodyReader = null; - } - public IBodyReader? TakeBodyReader() { var reader = _bodyReader; @@ -287,6 +275,68 @@ public void MarkRemoteClosed() IsRemoteClosed = true; } + /// + /// RFC 9113 §5.1 stream states, expressed relative to this endpoint: "local" is the direction + /// this endpoint sends on (request body for a client, response body for a server); "remote" is + /// the direction the peer sends on. Derived — not independently stored — from the existing + /// / / + /// fields so it can never drift from them. + /// + public enum StreamLifecycle + { + Open, + HalfClosedLocal, + HalfClosedRemote, + Closed + } + + /// + /// True once this endpoint's own send direction has nothing left in flight: either no drain was + /// ever started (a body sent entirely inline, or none at all) or a started drain has completed. + /// Note this is true by default (before is ever called) - a + /// brand-new stream is only "not locally done" while an outbound drain is actively pending, so + /// reports , not + /// , until is called. + /// + private bool IsLocalSendComplete => !HasBodyDrain || IsBodyDrainComplete; + + public StreamLifecycle Lifecycle => (IsRemoteClosed, IsLocalSendComplete) switch + { + (false, false) => StreamLifecycle.Open, + (false, true) => StreamLifecycle.HalfClosedLocal, + (true, false) => StreamLifecycle.HalfClosedRemote, + (true, true) => StreamLifecycle.Closed + }; + + /// + /// True once both directions have settled (RFC 9113 §5.1 "closed") — the only moment a session + /// manager may tear down this (remove from its stream map, return the + /// body reader, dispose). Callers that gate release on the drain flags should use this single + /// question instead of re-deriving the combination themselves. + /// + public bool MayRelease => Lifecycle == StreamLifecycle.Closed; + + /// + /// Records that the peer's direction has finished (RFC 9113 §5.1 END_STREAM received) and + /// returns whether the stream is now fully releasable per . + /// + public bool OnRemoteDone() + { + MarkRemoteClosed(); + return MayRelease; + } + + /// + /// Records that this endpoint's own outbound drain has finished (mirrors + /// ) and returns whether the stream is now fully releasable + /// per . + /// + public bool OnLocalDone() + { + MarkBodyDrainComplete(); + return MayRelease; + } + protected override void OnReset() { _headerOwner?.Dispose(); @@ -306,7 +356,6 @@ protected override void OnReset() _bufferedRemainder = default; HasBodyDrain = false; IsBodyDrainComplete = false; - IsBodyReadPending = false; ExpectedBodyLength = null; IsRemoteClosed = false; // Timer keys intentionally NOT cleared — they are stream-ID-derived strings that survive diff --git a/src/GaudiHTTP/Protocol/Syntax/Http3/Client/Http3ClientSessionManager.cs b/src/GaudiHTTP/Protocol/Syntax/Http3/Client/Http3ClientSessionManager.cs index 7a53009f..866fa745 100644 --- a/src/GaudiHTTP/Protocol/Syntax/Http3/Client/Http3ClientSessionManager.cs +++ b/src/GaudiHTTP/Protocol/Syntax/Http3/Client/Http3ClientSessionManager.cs @@ -28,15 +28,9 @@ internal sealed record AbandonedResponseBody(long StreamId); private readonly Http3ClientEncoder _requestEncoder; private readonly QpackTableSync _tableSync; - // Connection-level outbound credit for the multiplexed body pump. Each emitted DATA frame - // consumes one unit; the transport replenishes one unit per drained outbound item via - // OnOutboundFlushed. Caps the in-flight (emitted-but-unflushed) 16 KB frames per connection, - // keeping the shared array pool warm instead of exhausting it under concurrent uploads. - private const int OutboundBodyCapacity = 16; - private readonly Dictionary _drainContentOwners = new(); private readonly CancellationTokenSource _connectionCts = new(); - private MultiplexedBodyPump? _pump; + private readonly Http3OutboundWriter _writer; private bool _controlPrefaceSent; private bool _transportConnected; @@ -77,6 +71,9 @@ public Http3ClientSessionManager( { OnStreamClosedCallback = OnStreamClosed }; + + _writer = new Http3OutboundWriter( + this, _connectionCts, _options.ResolveRequestBodyChunkSize(_options.Http3)); } private void OnStreamClosed(long streamId) @@ -141,55 +138,91 @@ public void EncodeRequest(HttpRequestMessage request) var contentLength = request.Content?.Headers.ContentLength; var bodyStream = request.Content?.ReadAsStream(); - if (bodyStream is MemoryStream ms && ms.TryGetBuffer(out var segment)) + if (TryEmitMemoryStreamBody(streamId, bodyStream) + || TryEmitSerializedBodyDirect(streamId, request.Content!, contentLength) + || TryEmitEmptyBody(streamId, contentLength)) { - var pos = (int)ms.Position; - var available = segment.Count - pos; - if (available > 0) - { - var dataFrame = new DataFrame(segment.AsMemory(pos, available)); - EmitSerializedFrame(dataFrame, streamId); - EmitOutbound(new CompleteWrites(StreamTarget.FromId(streamId))); - return; - } + return; } - if (contentLength is > 0 and { } knownLength - && knownLength <= _options.ResolveMaxBufferedRequestBodySize(_options.Http3) - && TrySerializeBodyDirect(request.Content!, streamId, (int)knownLength)) + RegisterBodyPump(streamId, request.Content!, bodyStream!); + } + + /// + /// Fast path for a body already fully buffered in memory: writes it as a single DATA frame + /// directly from the underlying array, skipping the pump entirely. Returns false (falling + /// through to the next strategy) if the stream isn't a seekable in-memory buffer or has + /// nothing left to read at the current position. + /// + private bool TryEmitMemoryStreamBody(long streamId, Stream? bodyStream) + { + if (bodyStream is not MemoryStream ms || !ms.TryGetBuffer(out var segment)) { - return; + return false; } - if (contentLength == 0) + var pos = (int)ms.Position; + var available = segment.Count - pos; + if (available <= 0) { - // Empty body: emit END_STREAM directly without involving the pump (spec invariant 7). - EmitBufferedDataFrames(streamId, default, endStream: true); - return; + return false; } - var state = _streamManager.GetOrCreateStreamState(streamId); - state.MarkBodyDrainActive(); - _drainContentOwners[streamId] = request.Content!; - _pump ??= new MultiplexedBodyPump(this, _connectionCts, - _options.ResolveRequestBodyChunkSize(_options.Http3), OutboundBodyCapacity); - _pump.Register(streamId, bodyStream!, contentLength: null, CancellationToken.None); + var dataFrame = new DataFrame(segment.AsMemory(pos, available)); + EmitSerializedFrame(dataFrame, streamId); + EmitOutbound(new CompleteWrites(StreamTarget.FromId(streamId))); + return true; + } + + /// + /// Synchronously copies a known-length, buffer-size-bounded body into a pooled array and emits + /// it as a single DATA frame, avoiding the pump for small bodies whose content doesn't support + /// zero-copy access (e.g. non- content). + /// + private bool TryEmitSerializedBodyDirect(long streamId, HttpContent content, long? contentLength) + { + return contentLength is > 0 and { } knownLength + && knownLength <= _options.ResolveMaxBufferedRequestBodySize(_options.Http3) + && TrySerializeBodyDirect(content, streamId, (int)knownLength); + } + + /// + /// Emits END_STREAM directly for a declared-empty body without involving the pump + /// (spec invariant 7). + /// + private bool TryEmitEmptyBody(long streamId, long? contentLength) + { + if (contentLength != 0) + { + return false; + } + + EmitBufferedDataFrames(streamId, default, endStream: true); + return true; + } + + /// + /// Fallback strategy for a body of unknown or large length: registers it with the shared + /// multiplexed body pump for chunked, backpressure-aware draining. + /// + private void RegisterBodyPump(long streamId, HttpContent content, Stream bodyStream) + { + // Ensure the stream state is registered before the pump starts delivering completions. + _streamManager.GetOrCreateStreamState(streamId); + _drainContentOwners[streamId] = content; + _writer.Register(streamId, bodyStream, CancellationToken.None); } public void OnBodyMessage(object msg) { switch (msg) { - case BodyReadContinue cont: - _pump?.HandleBodyReadContinue(cont.StreamId); - break; - case BodyReadComplete read: - _pump?.HandleReadComplete(read.StreamId, read.BytesRead); + _writer.HandleReadComplete(read.StreamId, read.BytesRead); break; case BodyReadFailed failed: - _pump?.HandleReadFailed(failed.StreamId, failed.Reason); + _writer.HandleReadFailed(failed.StreamId, failed.Reason); break; case AbandonedResponseBody abandoned: @@ -212,28 +245,10 @@ public void OpenCriticalStreams() _controlPrefaceSent = true; - var settings = new Settings(); - settings.Set(SettingsIdentifier.QpackMaxTableCapacity, _encoderOptions.QpackMaxTableCapacity); - settings.Set(SettingsIdentifier.QpackBlockedStreams, _encoderOptions.QpackBlockedStreams); - settings.Set(SettingsIdentifier.MaxFieldSectionSize, _decoderOptions.MaxFieldSectionSize); - var settingsFrame = settings.ToFrame(); - - var streamTypeSize = QuicVarInt.EncodedLength((long)StreamType.Control); - var frameSize = settingsFrame.SerializedSize; - var totalSize = streamTypeSize + frameSize; - - using var owner = MemoryPool.Shared.Rent(totalSize); - var span = owner.Memory.Span; - - var written = QuicVarInt.Encode((long)StreamType.Control, span); - span = span[written..]; - settingsFrame.WriteTo(ref span); - - var buf = TransportBuffer.Rent(totalSize); - owner.Memory.Span[..totalSize].CopyTo(buf.FullMemory.Span); - buf.Length = totalSize; - - return MultiplexedData.Rent(buf, CriticalStreamId.Control); + return Http3OutboundWriter.BuildControlPreface( + _encoderOptions.QpackMaxTableCapacity, + _encoderOptions.QpackBlockedStreams, + _decoderOptions.MaxFieldSectionSize); } public IReadOnlyList DecodeServerData(TransportBuffer buffer, long streamId) @@ -292,7 +307,7 @@ public void OnTransportDisconnected() public void OnOutboundFlushed() { - _pump?.OnCapacityAvailable(); + _writer.OnCapacityAvailable(); } public IReadOnlyDictionary GetCorrelationMap() @@ -315,7 +330,7 @@ public bool TryCancelStream(HttpRequestMessage request) EmitOutbound(new ResetStream(streamId, 0x10C)); _streamManager.RemoveCorrelation(streamId); request.Fail(new OperationCanceledException("Request cancelled by caller.")); - _pump?.Cancel(streamId); + _writer.Cancel(streamId); _tracker.OnStreamClosed(streamId); return true; @@ -332,7 +347,7 @@ public void ResetConnectionState() public void Cleanup() { - _pump?.Cleanup(); + _writer.Cleanup(); _drainContentOwners.Clear(); _streamManager.Dispose(); @@ -415,22 +430,7 @@ private void EmitBufferedDataFrames(long streamId, ReadOnlyMemory body, bo return; } - var typeVarIntLen = QuicVarInt.EncodedLength((long)FrameType.Data); - var payloadVarIntLen = QuicVarInt.EncodedLength(body.Length); - var prefixSize = typeVarIntLen + payloadVarIntLen; - var totalWireSize = prefixSize + body.Length; - - var buf = TransportBuffer.Rent(totalWireSize); - var span = buf.FullMemory.Span; - - QuicVarInt.Encode((long)FrameType.Data, span); - span = span[typeVarIntLen..]; - QuicVarInt.Encode(body.Length, span); - span = span[payloadVarIntLen..]; - body.Span.CopyTo(span); - - buf.Length = totalWireSize; - EmitOutbound(MultiplexedData.Rent(buf, streamId)); + Http3OutboundWriter.EmitDataFrame(EmitOutbound, streamId, body); if (endStream) { @@ -442,11 +442,9 @@ void IMultiplexedBodyDrainTarget.OnDrainComplete(long streamId) { _drainContentOwners.Remove(streamId); - var state = _streamManager.TryGetStreamState(streamId); - if (state is not null) + if (_streamManager.TryGetStreamState(streamId) is not null) { Tracing.For("Protocol").Debug(this, "HTTP/3: request body complete (stream={0})", streamId); - state.MarkBodyDrainComplete(); } } diff --git a/src/GaudiHTTP/Protocol/Syntax/Http3/Client/StreamManager.cs b/src/GaudiHTTP/Protocol/Syntax/Http3/Client/StreamManager.cs index 55d10863..de181edb 100644 --- a/src/GaudiHTTP/Protocol/Syntax/Http3/Client/StreamManager.cs +++ b/src/GaudiHTTP/Protocol/Syntax/Http3/Client/StreamManager.cs @@ -79,17 +79,7 @@ public void FlushPendingResponse(long streamId) { if (_streams.TryGetValue(streamId, out var state) && state.HasBodyReader) { - state.FeedBody(ReadOnlySpan.Empty, endStream: true); - - if (state.TryTakeBufferedBodyReader(out var buffered)) - { - DispatchBufferedResponse(streamId, state, buffered!); - } - else - { - state.DetachBodyReader(); - } - + CompleteStreamOnFin(streamId, state); ReturnStreamState(streamId); return; } @@ -149,17 +139,7 @@ public void FlushAllPendingResponses() { if (state.HasBodyReader) { - state.FeedBody(ReadOnlySpan.Empty, endStream: true); - - if (state.TryTakeBufferedBodyReader(out var buffered)) - { - DispatchBufferedResponse(streamId, state, buffered!); - } - else - { - state.DetachBodyReader(); - } - + CompleteStreamOnFin(streamId, state); handledStreamIds.Add(streamId); } } @@ -195,76 +175,39 @@ public void ResolveBlockedStreams( { foreach (var (streamId, headers) in resolved) { - if (_streams.TryGetValue(streamId, out var state)) + if (!_streams.TryGetValue(streamId, out var state)) { - if (!state.HasResponse) - { - if (!responseDecoder.AssembleHeaders(headers, state) - && responseDecoder.LastInterimResponse is { } interim) - { - responseDecoder.LastInterimResponse = null; - interim.RequestMessage = _correlationMap.GetValueOrDefault(state.StreamId); - ops.OnResponse(interim); - } - } + continue; + } - if (state is { HasResponse: true, HasBodyReader: false }) + if (!state.HasResponse) + { + if (!responseDecoder.AssembleHeaders(headers, state) + && responseDecoder.LastInterimResponse is { } interim) { - var contentLength = state.PeekContentLength(); - if (contentLength is > 0 and var n && n <= maxBufferedResponseBodySize) - { - var buffered = ConnectionObjectPool.Instance.Rent(static () => new BufferedBodyReader()); - buffered.Reset((int)n); - state.InitBodyReader(buffered, maxResponseBodySize); - } - else - { - var queued = ConnectionObjectPool.Instance.Rent(() => new QueuedBodyReader(capacity: 8)); - state.InitBodyReader(queued, maxResponseBodySize); - var response = state.GetResponse(); - var stageActor = ops.StageActor; - var capturedId = streamId; - var bodyStream = queued.AsStream(onAbandoned: () => - stageActor.Tell(new Http3ClientSessionManager.AbandonedResponseBody(capturedId), ActorRefs.NoSender)); - response.Content = new StreamContent(bodyStream); - state.ApplyContentHeadersTo(response.Content); - - if (_correlationMap.Remove(streamId, out var request)) - { - response.RequestMessage = request; - } - - var partialContentResult = PartialContentValidator.Validate(response); - if (!partialContentResult.IsValid) - { - Tracing.For("Protocol").Warning(this, "{0}", partialContentResult.ErrorMessage!); - } - - ops.OnResponse(response); - } - - // Replay DATA buffered while the stream was blocked, then honor a FIN that - // arrived during the block so the body completes. - state.IsHeadersBlocked = false; - state.ReplayPendingInboundData(); - - if (state.PendingEndStream) - { - state.FeedBody(ReadOnlySpan.Empty, endStream: true); - - if (state.TryTakeBufferedBodyReader(out var pendingBuffered)) - { - DispatchBufferedResponse(streamId, state, pendingBuffered!); - } - else - { - state.DetachBodyReader(); - } - - ReturnStreamState(streamId); - } + responseDecoder.LastInterimResponse = null; + interim.RequestMessage = _correlationMap.GetValueOrDefault(state.StreamId); + ops.OnResponse(interim); } } + + if (state is not { HasResponse: true, HasBodyReader: false }) + { + continue; + } + + InitBodyReaderAndDispatch(streamId, state); + + // Replay DATA buffered while the stream was blocked, then honor a FIN that + // arrived during the block so the body completes. + state.IsHeadersBlocked = false; + state.ReplayPendingInboundData(); + + if (state.PendingEndStream) + { + CompleteStreamOnFin(streamId, state); + ReturnStreamState(streamId); + } } } @@ -407,7 +350,18 @@ private void HandleResponseHeaders(HeadersFrame frame, StreamState state) return; } - var streamId = state.StreamId; + InitBodyReaderAndDispatch(state.StreamId, state); + } + + /// + /// Initializes the response body reader for a stream whose headers just became available + /// (either assembled directly or unblocked by QPACK dynamic-table updates), dispatching the + /// response immediately unless the body is small enough to buffer in full — in which case + /// is deferred until the FIN epilogue (see + /// , ). + /// + private void InitBodyReaderAndDispatch(long streamId, StreamState state) + { var contentLength = state.PeekContentLength(); if (contentLength is > 0 and var n && n <= maxBufferedResponseBodySize) @@ -445,6 +399,27 @@ private void HandleResponseHeaders(HeadersFrame frame, StreamState state) ops.OnResponse(response); } + /// + /// FIN epilogue shared by every path that completes a stream's body: feeds an empty + /// end-of-stream chunk, then dispatches the buffered response (if the body was collected in + /// full) or detaches the reader for the streaming case. Does not remove the stream state — + /// callers that need to pool it call themselves, since some + /// (e.g. ) cannot mutate _streams mid-iteration. + /// + private void CompleteStreamOnFin(long streamId, StreamState state) + { + state.FeedBody(ReadOnlySpan.Empty, endStream: true); + + if (state.TryTakeBufferedBodyReader(out var buffered)) + { + DispatchBufferedResponse(streamId, state, buffered!); + } + else + { + state.DetachBodyReader(); + } + } + /// /// Completes a response whose body was buffered (Content-Length within the configured /// threshold): the body reader collected every DATA frame, so the final byte[] is materialized @@ -586,7 +561,7 @@ private void ReturnDecoder(long streamId) /// /// Callback invoked when a stream is closed (response emitted). - /// The StateMachine uses this to update and . + /// The SessionManager sets and uses this to update and . /// internal Action? OnStreamClosedCallback { get; init; } } diff --git a/src/GaudiHTTP/Protocol/Syntax/Http3/Http3OutboundWriter.cs b/src/GaudiHTTP/Protocol/Syntax/Http3/Http3OutboundWriter.cs new file mode 100644 index 00000000..727df9db --- /dev/null +++ b/src/GaudiHTTP/Protocol/Syntax/Http3/Http3OutboundWriter.cs @@ -0,0 +1,131 @@ +using System.Buffers; +using Servus.Akka.Transport; +using GaudiHTTP.Protocol.Body; + +namespace GaudiHTTP.Protocol.Syntax.Http3; + +/// +/// Owns the outbound wire-emission concerns shared by the HTTP/3 client and server session +/// managers: control-preface construction, DATA-frame framing, and the multiplexed body pump +/// together with its connection-level outbound credit. Client/server differences (throw-vs-null +/// preface handling, CompleteWrites/rate-observation on DATA emission) stay in the +/// session managers, which call into this writer for the shared mechanics. +/// +internal sealed class Http3OutboundWriter +{ + // Connection-level outbound credit for the multiplexed body pump. Each emitted DATA frame + // consumes one unit; the transport replenishes one unit per drained outbound item via + // OnOutboundFlushed. Caps the in-flight (emitted-but-unflushed) 16 KB frames per connection, + // keeping the shared array pool warm instead of exhausting it under concurrent uploads. + public const int OutboundBodyCapacity = 16; + + private readonly IMultiplexedBodyDrainTarget _target; + private readonly CancellationTokenSource _connectionCts; + private readonly int _bodyChunkSize; + private MultiplexedBodyPump? _pump; + + public Http3OutboundWriter( + IMultiplexedBodyDrainTarget target, CancellationTokenSource connectionCts, int bodyChunkSize) + { + _target = target; + _connectionCts = connectionCts; + _bodyChunkSize = bodyChunkSize; + } + + /// + /// Builds the control-stream preface (stream-type byte + SETTINGS frame). Stateless — callers + /// track their own "already sent" flag and decide whether a repeat call should return null + /// (client) or throw (server). + /// + public static MultiplexedData BuildControlPreface( + int qpackMaxTableCapacity, int qpackBlockedStreams, int maxFieldSectionSize) + { + var settings = new Settings(); + settings.Set(SettingsIdentifier.QpackMaxTableCapacity, qpackMaxTableCapacity); + settings.Set(SettingsIdentifier.QpackBlockedStreams, qpackBlockedStreams); + settings.Set(SettingsIdentifier.MaxFieldSectionSize, maxFieldSectionSize); + var settingsFrame = settings.ToFrame(); + + var streamTypeSize = QuicVarInt.EncodedLength((long)StreamType.Control); + var frameSize = settingsFrame.SerializedSize; + var totalSize = streamTypeSize + frameSize; + + using var owner = MemoryPool.Shared.Rent(totalSize); + var span = owner.Memory.Span; + + var written = QuicVarInt.Encode((long)StreamType.Control, span); + span = span[written..]; + settingsFrame.WriteTo(ref span); + + var buf = TransportBuffer.Rent(totalSize); + owner.Memory.Span[..totalSize].CopyTo(buf.FullMemory.Span); + buf.Length = totalSize; + + return MultiplexedData.Rent(buf, CriticalStreamId.Control); + } + + /// + /// Encodes a DATA frame for and hands it to . + /// No-op for an empty body — callers decide separately whether to still emit + /// CompleteWrites for the end-of-stream case. + /// + public static void EmitDataFrame(Action emit, long streamId, ReadOnlyMemory body) + { + if (body.IsEmpty) + { + return; + } + + var typeVarIntLen = QuicVarInt.EncodedLength((long)FrameType.Data); + var payloadVarIntLen = QuicVarInt.EncodedLength(body.Length); + var prefixSize = typeVarIntLen + payloadVarIntLen; + var totalWireSize = prefixSize + body.Length; + + var buf = TransportBuffer.Rent(totalWireSize); + var span = buf.FullMemory.Span; + + QuicVarInt.Encode((long)FrameType.Data, span); + span = span[typeVarIntLen..]; + QuicVarInt.Encode(body.Length, span); + span = span[payloadVarIntLen..]; + body.Span.CopyTo(span); + + buf.Length = totalWireSize; + emit(MultiplexedData.Rent(buf, streamId)); + } + + /// + /// Lazily creates the shared multiplexed body pump (bounded by ) + /// and registers a stream's body for draining. + /// + public void Register(long streamId, Stream bodyStream, CancellationToken cancellationToken) + { + _pump ??= new MultiplexedBodyPump(_target, _connectionCts, _bodyChunkSize, OutboundBodyCapacity); + _pump.Register(streamId, bodyStream, contentLength: null, cancellationToken); + } + + public void HandleReadComplete(long streamId, int bytesRead) + { + _pump?.HandleReadComplete(streamId, bytesRead); + } + + public void HandleReadFailed(long streamId, Exception reason) + { + _pump?.HandleReadFailed(streamId, reason); + } + + public void OnCapacityAvailable() + { + _pump?.OnCapacityAvailable(); + } + + public void Cancel(long streamId) + { + _pump?.Cancel(streamId); + } + + public void Cleanup() + { + _pump?.Cleanup(); + } +} diff --git a/src/GaudiHTTP/Protocol/Syntax/Http3/Server/Http3ServerSessionManager.cs b/src/GaudiHTTP/Protocol/Syntax/Http3/Server/Http3ServerSessionManager.cs index 6dd3d583..9a5e6d2f 100644 --- a/src/GaudiHTTP/Protocol/Syntax/Http3/Server/Http3ServerSessionManager.cs +++ b/src/GaudiHTTP/Protocol/Syntax/Http3/Server/Http3ServerSessionManager.cs @@ -1,4 +1,3 @@ -using System.Buffers; using Akka.Actor; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; @@ -18,8 +17,6 @@ namespace GaudiHTTP.Protocol.Syntax.Http3.Server; internal sealed class Http3ServerSessionManager : IMultiplexedBodyDrainTarget { - private const int MaxStatePoolCapacity = 1000; - private const string DataRateCheck = "data-rate-check"; private readonly IServerStageOperations _ops; @@ -36,14 +33,9 @@ internal sealed class Http3ServerSessionManager : IMultiplexedBodyDrainTarget private readonly int _responseBodyChunkSize; private readonly Dictionary _streams = new(); - // Connection-level outbound credit for the multiplexed response-body pump. Each emitted DATA - // frame consumes one unit; the transport replenishes one unit per drained outbound item via - // OnOutboundFlushed. Bounds in-flight (emitted-but-unflushed) 16 KB frames per connection so - // concurrent responses cannot flood the per-stream output pipes and exhaust the shared pool. - private const int OutboundBodyCapacity = 16; private readonly CancellationTokenSource _connectionCts = new(); - private MultiplexedBodyPump? _pump; + private readonly Http3OutboundWriter _writer; private readonly DataRateMonitor _requestRate; private readonly DataRateMonitor _responseRate; private readonly List _rateViolations = []; @@ -96,9 +88,7 @@ public Http3ServerSessionManager( _requestRate = new DataRateMonitor(rate.MinRequestBodyDataRate, rate.MinRequestBodyDataRateGracePeriod); _responseRate = new DataRateMonitor(rate.MinResponseDataRate, rate.MinResponseDataRateGracePeriod); - var statePoolCapacity = Math.Min( - _decoderOptions.MaxConcurrentStreams > 0 ? _decoderOptions.MaxConcurrentStreams : 100, - MaxStatePoolCapacity); + _writer = new Http3OutboundWriter(this, _connectionCts, _responseBodyChunkSize); } public void PreStart() @@ -233,9 +223,7 @@ public void OnResponse(IFeatureCollection features) } var bodyStream = gaudiBody.GetResponseStream(); - state.MarkBodyDrainActive(); - _pump ??= new MultiplexedBodyPump(this, _connectionCts, _responseBodyChunkSize, OutboundBodyCapacity); - _pump.Register(streamId, bodyStream, contentLength: null, CancellationToken.None); + _writer.Register(streamId, bodyStream, CancellationToken.None); Tracing.For("Protocol").Debug(this, "HTTP/3: response body drain started (stream={0})", streamId); } @@ -262,23 +250,19 @@ public void OnBodyMessage(object msg) { switch (msg) { - case BodyReadContinue cont: - _pump?.HandleBodyReadContinue(cont.StreamId); - break; - case BodyReadComplete read: - _pump?.HandleReadComplete(read.StreamId, read.BytesRead); + _writer.HandleReadComplete(read.StreamId, read.BytesRead); break; case BodyReadFailed failed: - _pump?.HandleReadFailed(failed.StreamId, failed.Reason); + _writer.HandleReadFailed(failed.StreamId, failed.Reason); break; } } public void OnOutboundFlushed() { - _pump?.OnCapacityAvailable(); + _writer.OnCapacityAvailable(); } public void FlushAllPendingRequests() @@ -292,7 +276,7 @@ public void FlushAllPendingRequests() public void Cleanup() { - _pump?.Cleanup(); + _writer.Cleanup(); foreach (var (_, (decoder, state)) in _streams) { @@ -515,33 +499,7 @@ private void ProcessFrameData(TransportBuffer buffer, long streamId) { case HeadersFrame headersFrame: { - if (state.GetRequestFeature() is not null) - { - _requestDecoder.DecodeTrailers(headersFrame, state); - state.FeedBody([], endStream: true); - } - else - { - var requestFeature = - _requestDecoder.DecodeHeadersToFeature(headersFrame, state, endStream: false); - if (requestFeature is not null) - { - state.InitRequestFeature(requestFeature); - } - else - { - if (state.GetRequestFeature() is null) - { - // QPACK-blocked: the header block is queued in the table sync - // awaiting encoder-stream instructions. Mark it so the FIN is - // deferred and ProcessQpackEncoderStream redrives dispatch. - state.IsHeadersBlocked = true; - } - - _ops.OnScheduleTimer(state.HeadersTimeoutTimerKey, _requestHeadersTimeout); - } - } - + HandleHeadersFrame(state, headersFrame); break; } @@ -563,17 +521,11 @@ private void ProcessFrameData(TransportBuffer buffer, long streamId) } } } - catch (QpackException ex) - { - Tracing.For("Protocol").Warning(this, - "HTTP/3 QPACK error on stream {0} - closing connection: {1}", streamId, ex.Message); - ShouldComplete = true; - return; - } - catch (HuffmanException ex) + catch (Exception ex) when (ex is QpackException or HuffmanException) { + var kind = ex is QpackException ? "QPACK" : "Huffman"; Tracing.For("Protocol").Warning(this, - "HTTP/3 Huffman error on stream {0} - closing connection: {1}", streamId, ex.Message); + "HTTP/3 {0} error on stream {1} - closing connection: {2}", kind, streamId, ex.Message); ShouldComplete = true; return; } @@ -593,6 +545,38 @@ private void ProcessFrameData(TransportBuffer buffer, long streamId) } } + /// + /// Triages a HEADERS frame on a request stream: trailers if a request feature already + /// exists, otherwise a request-headers decode that either dispatches, blocks on QPACK + /// (until the encoder stream resolves it), or is still awaiting continuation. + /// + private void HandleHeadersFrame(StreamState state, HeadersFrame headersFrame) + { + if (state.GetRequestFeature() is not null) + { + _requestDecoder.DecodeTrailers(headersFrame, state); + state.FeedBody([], endStream: true); + return; + } + + var requestFeature = _requestDecoder.DecodeHeadersToFeature(headersFrame, state, endStream: false); + if (requestFeature is not null) + { + state.InitRequestFeature(requestFeature); + return; + } + + if (state.GetRequestFeature() is null) + { + // QPACK-blocked: the header block is queued in the table sync + // awaiting encoder-stream instructions. Mark it so the FIN is + // deferred and ProcessQpackEncoderStream redrives dispatch. + state.IsHeadersBlocked = true; + } + + _ops.OnScheduleTimer(state.HeadersTimeoutTimerKey, _requestHeadersTimeout); + } + private void HandleSettingsFrame(SettingsFrame settings) { if (_settingsReceived) @@ -725,7 +709,7 @@ private void HandleDataFrame(DataFrame dataFrame, long streamId, StreamState sta } } - private long GetStreamIdFromFeatures(IFeatureCollection features) + private static long GetStreamIdFromFeatures(IFeatureCollection features) { var streamIdFeature = features.Get(); if (streamIdFeature is not null) @@ -740,7 +724,7 @@ private void CloseStream(long streamId) { _requestRate.Remove(streamId); _responseRate.Remove(streamId); - _pump?.Cancel(streamId); + _writer.Cancel(streamId); if (_streams.TryGetValue(streamId, out var streamData)) { @@ -777,7 +761,6 @@ void IMultiplexedBodyDrainTarget.OnDrainComplete(long streamId) if (_streams.TryGetValue(streamId, out var streamData)) { - streamData.State.MarkBodyDrainComplete(); EmitEndOfBody(streamId, streamData.State); CloseStream(streamId); } @@ -846,29 +829,13 @@ private void EmitBufferedDataFrames(long streamId, ReadOnlyMemory body, bo return; } - var typeVarIntLen = QuicVarInt.EncodedLength((long)FrameType.Data); - var payloadVarIntLen = QuicVarInt.EncodedLength(body.Length); - var prefixSize = typeVarIntLen + payloadVarIntLen; - var totalWireSize = prefixSize + body.Length; - - var buf = TransportBuffer.Rent(totalWireSize); - var span = buf.FullMemory.Span; - - QuicVarInt.Encode((long)FrameType.Data, span); - span = span[typeVarIntLen..]; - QuicVarInt.Encode(body.Length, span); - span = span[payloadVarIntLen..]; - body.Span.CopyTo(span); - - buf.Length = totalWireSize; + Http3OutboundWriter.EmitDataFrame(_ops.OnOutbound, streamId, body); Tracing.For("Protocol").Trace(this, "HTTP/3: DATA out (stream={0}, len={1}, endStream={2})", streamId, body.Length, endStream); _responseRate.Observe(streamId, body.Length, Now()); EnsureRateTimer(); - - _ops.OnOutbound(MultiplexedData.Rent(buf, streamId)); } private MultiplexedData BuildControlPreface() @@ -880,30 +847,12 @@ private MultiplexedData BuildControlPreface() _controlPrefaceSent = true; - var settings = new Settings(); - settings.Set(SettingsIdentifier.QpackMaxTableCapacity, _encoderOptions.QpackMaxTableCapacity); - settings.Set(SettingsIdentifier.QpackBlockedStreams, _encoderOptions.QpackBlockedStreams); // RFC 9114 §7.2.4.1: advertise the largest header section we will accept so the peer can // pre-trim oversized header blocks instead of having them rejected after the fact. - settings.Set(SettingsIdentifier.MaxFieldSectionSize, _decoderOptions.MaxFieldSectionSize); - var settingsFrame = settings.ToFrame(); - - var streamTypeSize = QuicVarInt.EncodedLength((long)StreamType.Control); - var frameSize = settingsFrame.SerializedSize; - var totalSize = streamTypeSize + frameSize; - - using var owner = MemoryPool.Shared.Rent(totalSize); - var span = owner.Memory.Span; - - var written = QuicVarInt.Encode((long)StreamType.Control, span); - span = span[written..]; - settingsFrame.WriteTo(ref span); - - var buf = TransportBuffer.Rent(totalSize); - owner.Memory.Span[..totalSize].CopyTo(buf.FullMemory.Span); - buf.Length = totalSize; - - return MultiplexedData.Rent(buf, CriticalStreamId.Control); + return Http3OutboundWriter.BuildControlPreface( + _encoderOptions.QpackMaxTableCapacity, + _encoderOptions.QpackBlockedStreams, + _decoderOptions.MaxFieldSectionSize); } private void EnsureRateTimer() diff --git a/src/GaudiHTTP/Protocol/Syntax/Http3/StreamState.cs b/src/GaudiHTTP/Protocol/Syntax/Http3/StreamState.cs index 00d3c0c0..2c5caccd 100644 --- a/src/GaudiHTTP/Protocol/Syntax/Http3/StreamState.cs +++ b/src/GaudiHTTP/Protocol/Syntax/Http3/StreamState.cs @@ -34,12 +34,6 @@ internal sealed class StreamState : Poolable public bool HasBodyReader => _bodyReader is not null; - public bool HasBodyDrain { get; private set; } - - public bool IsBodyDrainComplete { get; private set; } - - public bool IsBodyReadPending { get; set; } - /// /// RFC 9204 §2.1.2 — true while inbound HEADERS are QPACK-blocked awaiting dynamic-table /// updates. DATA frames received in this window are buffered (not dropped) and replayed @@ -286,17 +280,6 @@ public void AbortBody() _bodyReader?.Dispose(); } - public void MarkBodyDrainActive() - { - HasBodyDrain = true; - IsBodyDrainComplete = false; - } - - public void MarkBodyDrainComplete() - { - IsBodyDrainComplete = true; - } - public void SetFeatures(IFeatureCollection features) { _features = features; @@ -319,9 +302,6 @@ protected override void OnReset() _bodyReader = null; _maxBodySize = 0; _totalBodyBytes = 0; - HasBodyDrain = false; - IsBodyDrainComplete = false; - IsBodyReadPending = false; IsHeadersBlocked = false; PendingEndStream = false; _pendingInboundData = null; diff --git a/src/GaudiHTTP/Streams/Lifecycle/ClientStreamOwner.cs b/src/GaudiHTTP/Streams/Lifecycle/ClientStreamOwner.cs index 42df101f..c754d1e0 100644 --- a/src/GaudiHTTP/Streams/Lifecycle/ClientStreamOwner.cs +++ b/src/GaudiHTTP/Streams/Lifecycle/ClientStreamOwner.cs @@ -53,7 +53,6 @@ private TimeSpan CalculateBackoff(int attempt) => private Source? _responseFanoutSource; private Sink? _requestIngress; private SharedKillSwitch? _killSwitch; - private bool _streamRunning; // Attached consumers in registration order. A consumer's PartitionHub slot is its 0-based // index here; the list compacts on unregister so indices track the hub's live partition set. private readonly List _attachedConsumers = []; @@ -208,7 +207,6 @@ private void MaterializeStream() _responseFanoutSource = fanoutSource; - _streamRunning = true; Tracing.For("Request").Debug(this, "Pipeline ready"); _log.Debug("Stream pipeline materialized successfully"); @@ -439,7 +437,6 @@ private void CleanupResources() _responseFanoutSource = null; _requestIngress = null; _attachedConsumers.Clear(); - _streamRunning = false; } protected override SupervisorStrategy SupervisorStrategy() @@ -453,7 +450,7 @@ protected override SupervisorStrategy SupervisorStrategy() protected override void PostStop() { - _log.Debug("PostStop: cleaning up resources (streamRunning: {0})", _streamRunning); + _log.Debug("PostStop: cleaning up resources"); Timers.CancelAll(); CleanupResources(); base.PostStop(); diff --git a/src/GaudiHTTP/Streams/Lifecycle/ServerSupervisorActor.cs b/src/GaudiHTTP/Streams/Lifecycle/ServerSupervisorActor.cs index 9aedf6fc..edd7e3db 100644 --- a/src/GaudiHTTP/Streams/Lifecycle/ServerSupervisorActor.cs +++ b/src/GaudiHTTP/Streams/Lifecycle/ServerSupervisorActor.cs @@ -134,8 +134,7 @@ private void OnListenerTerminated(Terminated msg) Timers.Cancel(StartupTimerKey); _log.Error("Listener {0} died during startup", msg.ActorRef.Path.Name); - foreach (var listener in _listenerActors.Where(x => - !_listenerActors.Any(actorRef => actorRef.Equals(msg.ActorRef)))) + foreach (var listener in _listenerActors.Where(x => !x.Equals(msg.ActorRef))) { Context.Stop(listener); } diff --git a/src/GaudiHTTP/Streams/Stages/Client/HttpClientConnectionStageLogic.cs b/src/GaudiHTTP/Streams/Stages/Client/HttpClientConnectionStageLogic.cs index febea56a..4e651035 100644 --- a/src/GaudiHTTP/Streams/Stages/Client/HttpClientConnectionStageLogic.cs +++ b/src/GaudiHTTP/Streams/Stages/Client/HttpClientConnectionStageLogic.cs @@ -59,10 +59,9 @@ public HttpClientConnectionStageLogic( return; } - if (!_sm.ShouldPauseNetwork && !HasBeenPulled(_inNetwork) && !IsClosed(_inNetwork)) + if (TryPullNetwork()) { Tracing.For(TraceCategory).Debug(this, "response outlet pull → pulling _inNetwork"); - Pull(_inNetwork); } }, onDownstreamFinish: cause => @@ -147,10 +146,9 @@ private void OnStageActorMessage((IActorRef sender, object message) args) Tracing.For(TraceCategory) .Debug(this, "after msg: pause={0}, pulled={1}, closed={2}", pauseAfter, pulled, closed); - if (!pauseAfter && !pulled && !closed) + if (TryPullNetwork()) { Tracing.For(TraceCategory).Debug(this, "re-pull _inNetwork after body message"); - Pull(_inNetwork); } TryPullRequest(); @@ -175,10 +173,7 @@ private void OnNetworkPush() TryPushResponse(); } - if (!_sm.ShouldPauseNetwork && !HasBeenPulled(_inNetwork) && !IsClosed(_inNetwork)) - { - Pull(_inNetwork); - } + TryPullNetwork(); TryPullRequest(); TryCompleteAfterAllResponses(); @@ -206,11 +201,7 @@ protected override void OnTimer(object timerKey) if (name == DrainCompleteTimerKey) { - if (IsClosed(_inRequest) - && !_sm.HasInFlightRequests - && !_sm.IsReconnecting - && _responseQueue.Count == 0 - && _outboundQueue.Count == 0) + if (IsFullyDrained) { Tracing.For(TraceCategory).Debug(this, "drain complete — closing stage"); CompleteStage(); @@ -286,19 +277,47 @@ private void TryPullRequest() } } + /// + /// True once every driver of stage completion has quiesced: no more requests can arrive, + /// nothing is in flight or reconnecting, and both queues are empty. + /// + private bool IsFullyDrained => + IsClosed(_inRequest) + && !_sm.HasInFlightRequests + && !_sm.IsReconnecting + && _responseQueue.Count == 0 + && _outboundQueue.Count == 0; + private void TryCompleteAfterAllResponses() { - if (IsClosed(_inRequest) - && !_sm.HasInFlightRequests - && !_sm.IsReconnecting - && _responseQueue.Count == 0 - && _outboundQueue.Count == 0 - && !IsTimerActive(DrainCompleteTimerKey)) + if (IsFullyDrained && !IsTimerActive(DrainCompleteTimerKey)) { + // Do not CompleteStage() synchronously here: pump reads for the last + // response's streaming body (FlowControlledBodyPump/MultiplexedBodyPump/ + // SerialBodyPump) complete off the stage thread and post their result back + // via IClientStageOperations.StageActor.Tell(...) (see SerialBodyPump.cs, + // PumpSlotLifecycle.StartRead). Such a message can already be in the actor's + // mailbox — queued behind the event that made all five conditions true — even + // though HasInFlightRequests/queue counts read as fully drained right now. This + // settle window gives that in-flight StageActor message a chance to be + // processed (re-arming state, e.g. HasInFlightRequests) before we tear the + // stage down; hard-completing immediately would deliver that message to a + // stopped stage actor and silently drop it. ScheduleOnce(DrainCompleteTimerKey, TimeSpan.FromMilliseconds(100)); } } + private bool TryPullNetwork() + { + if (!_sm.ShouldPauseNetwork && !HasBeenPulled(_inNetwork) && !IsClosed(_inNetwork)) + { + Pull(_inNetwork); + return true; + } + + return false; + } + private void CloseAllPorts() { _sm.OnUpstreamFinished(); diff --git a/src/GaudiHTTP/Streams/Stages/Routing/GroupByRequestEndpointStage.cs b/src/GaudiHTTP/Streams/Stages/Routing/GroupByRequestEndpointStage.cs index bf8f3598..f4d35ac8 100644 --- a/src/GaudiHTTP/Streams/Stages/Routing/GroupByRequestEndpointStage.cs +++ b/src/GaudiHTTP/Streams/Stages/Routing/GroupByRequestEndpointStage.cs @@ -273,8 +273,12 @@ public override void PreStart() if (_upstreamFinished) { + // TryFinish() unconditionally calls TryCompleteStage() on the path where + // every live slot has drained; its early-return path ("still draining") + // only fires when a live slot still has Pending items, in which case a + // follow-up TryCompleteStage() call would defer for that same reason — so + // no separate call is needed here. TryFinish(); - TryCompleteStage(); } else if (!HasBeenPulled(_stage._in) && !IsClosed(_stage._in)) { @@ -374,13 +378,9 @@ private void HandleOutPull() if (_pendingSources.TryDequeue(out var bufferedSource)) { Push(_stage._out, bufferedSource); - - if (!HasBeenPulled(_stage._in) && !IsClosed(_stage._in)) - { - Pull(_stage._in); - } } - else if (!HasBeenPulled(_stage._in) && !IsClosed(_stage._in)) + + if (!HasBeenPulled(_stage._in) && !IsClosed(_stage._in)) { Pull(_stage._in); } @@ -552,8 +552,9 @@ private void HandleDeadSlot(RequestEndpoint key, SubflowGroup group, SubflowStat if (_upstreamFinished) { + // See the comment in _onWriteReady: TryFinish() already reaches + // TryCompleteStage() on every path where completion is actually possible. TryFinish(); - TryCompleteStage(); } return; @@ -599,6 +600,11 @@ private void TransferPendingItems(RequestEndpoint key, SubflowGroup group, Subfl } } + // DrainPending -> HandleDeadSlot -> TransferPendingItems -> DrainPending forms a mutual + // recursion; it is bounded because each cycle either drains items into a live channel + // (shrinking Pending) or, on hitting a dead slot, permanently removes that slot from the + // group (RemoveSlot) before recursing — so the number of dead-slot hops is capped by the + // group's slot count and cannot cycle indefinitely. private void DrainPending(RequestEndpoint key, SubflowState state) { while (state.Pending.Count > 0) diff --git a/src/GaudiHTTP/Streams/Stages/Server/ApplicationBridgeStage.cs b/src/GaudiHTTP/Streams/Stages/Server/ApplicationBridgeStage.cs index cb70c89b..9ef1ee3f 100644 --- a/src/GaudiHTTP/Streams/Stages/Server/ApplicationBridgeStage.cs +++ b/src/GaudiHTTP/Streams/Stages/Server/ApplicationBridgeStage.cs @@ -55,6 +55,16 @@ private sealed class Logic : TimerGraphStageLogic private const string SoftTimerPrefix = "soft:"; private const string HardTimerPrefix = "hard:"; + private sealed class RequestSlot + { + public required IFeatureCollection Features { get; init; } + public TContext? AppContext { get; set; } + public CancellationTokenSource? Cts { get; set; } + public string? SoftTimerKey { get; set; } + public string? HardTimerKey { get; set; } + public bool InGracePhase { get; set; } + } + private readonly ApplicationBridgeStage _stage; private IActorRef? _stageActor; private bool _upstreamFinished; @@ -62,11 +72,7 @@ private sealed class Logic : TimerGraphStageLogic private int _sequence; private bool _downstreamReady; private readonly Queue _pending = new(); - private readonly Dictionary _activeTimeouts = []; - private readonly Dictionary _activeFeatures = []; - private readonly HashSet _gracePhase = []; - private readonly Dictionary _appContexts = []; - private readonly Dictionary _timerKeys = []; + private readonly Dictionary _requestSlots = []; private readonly bool _metricsEnabled; private readonly int _backpressureThreshold; private bool _backpressureSignaled; @@ -89,10 +95,7 @@ public Logic(ApplicationBridgeStage stage) : base(stage.Shape) { Tracing.For("Handler").Info(this, "bridge upstream finished (protocol completed), inFlight={0}", _inFlight); _upstreamFinished = true; - if (_inFlight == 0) - { - CompleteStage(); - } + TryCompleteIfDrained(); }, onUpstreamFailure: ex => { @@ -146,70 +149,43 @@ protected override void OnTimer(object timerKey) private void OnSoftTimeout(int seq) { - if (!_activeTimeouts.TryGetValue(seq, out var cts)) + if (!_requestSlots.TryGetValue(seq, out var slot)) { return; } - cts.Cancel(); - _gracePhase.Add(seq); - if (_timerKeys.TryGetValue(seq, out var keys)) + slot.Cts?.Cancel(); + slot.InGracePhase = true; + if (slot.HardTimerKey is not null) { - ScheduleOnce(keys.Hard, _stage._handlerGracePeriod); + ScheduleOnce(slot.HardTimerKey, _stage._handlerGracePeriod); } } private void OnHardTimeout(int seq) { - if (!_activeTimeouts.ContainsKey(seq) || !_gracePhase.Contains(seq)) - { - return; - } - - if (!_activeFeatures.TryGetValue(seq, out var features)) + if (!_requestSlots.TryGetValue(seq, out var slot) || !slot.InGracePhase) { return; } - CleanupTimeout(seq); - _inFlight--; - if (_metricsEnabled) - { - Metrics.HandlerTimeouts().Add(1); - Metrics.PipelineInFlight().Add(-1); - ResetBackpressure(); - } - - DisposeAppContext(seq, null); + var features = slot.Features; + // Only emit when the response has not already gone out. A streaming handler whose headers + // were emitted (ResponseReady's still-running branch) already pushed these features once; + // re-emitting here would deliver the same response twice (double OnResponse / wire + // corruption / double in-flight accounting). Completing the body (inside FinishRequest) + // ends the stalled stream; the late HandlerFinished is swallowed because the timeout entry + // is gone. var alreadyStarted = features.Get() is GaudiHttpResponseBodyFeature { HasStarted: true }; - if (!alreadyStarted) - { - var responseFeature = features.Get(); - responseFeature?.StatusCode = 503; - } - - CompleteResponseBody(features); - FireOnCompleted(features); - - // Only emit when the response has not already gone out. A streaming handler whose headers - // were emitted (ResponseReady's still-running branch) already pushed these features once; - // re-emitting here would deliver the same response twice (double OnResponse / wire - // corruption / double in-flight accounting). Completing the body above ends the stalled - // stream; the late HandlerFinished is swallowed because the timeout entry is gone. - if (!alreadyStarted) - { - Emit(features); - } + FinishRequest(seq, features, error: null, failStatus: alreadyStarted ? null : 503, + emit: !alreadyStarted, timedOut: true); - if (_upstreamFinished && _inFlight == 0) - { - CompleteStage(); - } + TryCompleteIfDrained(); } private void OnPush() @@ -230,17 +206,8 @@ private void OnPush() } catch (Exception) { - _inFlight--; - if (_metricsEnabled) - { - Metrics.PipelineInFlight().Add(-1); - } - - var responseFeature = features.Get(); - responseFeature?.StatusCode = 500; - CompleteResponseBody(features); - FireOnCompleted(features); - Emit(features); + FinishRequest(seq, features, error: null, failStatus: 500, emit: true, + cleanupSlot: false, resetBackpressure: false); } TryPullNext(); @@ -252,40 +219,26 @@ private void DispatchAsync(IFeatureCollection features, int seq) try { appContext = _stage._application.CreateContext(ContainerFor(features)); - _appContexts[seq] = appContext; } catch (Exception) { - _inFlight--; - var responseFeature = features.Get(); - responseFeature?.StatusCode = 500; - CompleteResponseBody(features); - FireOnCompleted(features); - Emit(features); + FinishRequest(seq, features, error: null, failStatus: 500, emit: true, + cleanupSlot: false, trackMetrics: false); return; } + var slot = new RequestSlot { Features = features, AppContext = appContext }; + _requestSlots[seq] = slot; + var task = _stage._application.ProcessRequestAsync(appContext); if (task.IsCompletedSuccessfully) { - _inFlight--; - _stage._application.DisposeContext(appContext, null); - _appContexts.Remove(seq); - CompleteResponseBody(features); - FireOnCompleted(features); - Emit(features); + FinishRequest(seq, features, error: null, failStatus: null, emit: true, trackMetrics: false); } else if (task.IsFaulted) { - _inFlight--; - var responseFeature = features.Get(); - responseFeature?.StatusCode = 500; - _stage._application.DisposeContext(appContext, task.Exception); - _appContexts.Remove(seq); - CompleteResponseBody(features); - FireOnCompleted(features); - Emit(features); + FinishRequest(seq, features, error: task.Exception, failStatus: 500, emit: true, trackMetrics: false); } else { @@ -303,9 +256,9 @@ private void DispatchAsync(IFeatureCollection features, int seq) HardTimerPrefix.AsSpan().CopyTo(span); s.TryFormat(span[HardTimerPrefix.Length..], out _); }); - _timerKeys[seq] = (softKey, hardKey); - _activeTimeouts[seq] = cts; - _activeFeatures[seq] = features; + slot.Cts = cts; + slot.SoftTimerKey = softKey; + slot.HardTimerKey = hardKey; ScheduleOnce(softKey, _stage._handlerTimeout); var bodyFeature = features.Get() as GaudiHttpResponseBodyFeature; @@ -331,30 +284,15 @@ private void OnMessage((IActorRef sender, object msg) args) switch (args.msg) { case ResponseReady(var seq, var features, var handlerTask): - if (handlerTask.IsFaulted && - features.Get() is not GaudiHttpResponseBodyFeature - { - HasStarted: true - }) - { - var responseFeature = features.Get(); - responseFeature?.StatusCode = 500; - } - if (handlerTask.IsCompleted) { - CompleteResponseBody(features); - FireOnCompleted(features); - _inFlight--; - if (_metricsEnabled) + var notStarted = features.Get() is not GaudiHttpResponseBodyFeature { - Metrics.PipelineInFlight().Add(-1); - ResetBackpressure(); - } + HasStarted: true + }; - CleanupTimeout(seq); - DisposeAppContext(seq, handlerTask.Exception); - Emit(features); + FinishRequest(seq, features, error: handlerTask.Exception, + failStatus: handlerTask.IsFaulted && notStarted ? 500 : null, emit: true); } else { @@ -367,93 +305,41 @@ private void OnMessage((IActorRef sender, object msg) args) break; case HandlerFinished(var seq, var finishedFeatures): - if (!_activeTimeouts.ContainsKey(seq)) + if (!_requestSlots.ContainsKey(seq)) { break; } - CompleteResponseBody(finishedFeatures); - FireOnCompleted(finishedFeatures); - _inFlight--; - if (_metricsEnabled) - { - Metrics.PipelineInFlight().Add(-1); - ResetBackpressure(); - } - - CleanupTimeout(seq); - DisposeAppContext(seq, null); - if (_upstreamFinished && _inFlight == 0) - { - CompleteStage(); - } - + FinishRequest(seq, finishedFeatures, error: null, failStatus: null, emit: false); + TryCompleteIfDrained(); break; case HandlerFaulted(var seq, var faultedFeatures, var error): - if (!_activeTimeouts.ContainsKey(seq)) + if (!_requestSlots.ContainsKey(seq)) { break; } - CompleteResponseBody(faultedFeatures); - FireOnCompleted(faultedFeatures); - _inFlight--; - if (_metricsEnabled) - { - Metrics.PipelineInFlight().Add(-1); - ResetBackpressure(); - } - - CleanupTimeout(seq); - DisposeAppContext(seq, error); - if (_upstreamFinished && _inFlight == 0) - { - CompleteStage(); - } - + FinishRequest(seq, faultedFeatures, error: error, failStatus: null, emit: false); + TryCompleteIfDrained(); break; case DispatchCompleted(var seq, var features): - if (!_activeTimeouts.ContainsKey(seq)) + if (!_requestSlots.ContainsKey(seq)) { break; } - _inFlight--; - if (_metricsEnabled) - { - Metrics.PipelineInFlight().Add(-1); - ResetBackpressure(); - } - - CleanupTimeout(seq); - DisposeAppContext(seq, null); - CompleteResponseBody(features); - FireOnCompleted(features); - Emit(features); + FinishRequest(seq, features, error: null, failStatus: null, emit: true); break; case DispatchFailed(var seq, var features, var error): - if (!_activeTimeouts.ContainsKey(seq)) + if (!_requestSlots.ContainsKey(seq)) { break; } - _inFlight--; - if (_metricsEnabled) - { - Metrics.PipelineInFlight().Add(-1); - ResetBackpressure(); - } - - CleanupTimeout(seq); - DisposeAppContext(seq, error); - var respFeature = features.Get(); - respFeature?.StatusCode = 500; - CompleteResponseBody(features); - FireOnCompleted(features); - Emit(features); + FinishRequest(seq, features, error: error, failStatus: 500, emit: true); break; } @@ -463,6 +349,67 @@ private void OnMessage((IActorRef sender, object msg) args) } } + // Owns the complete per-request finish sequence: optional fail-status stamping, response-body + // completion, OnCompleted firing, in-flight/metrics bookkeeping, request-slot teardown + // (timers + CTS + app context disposal), and the optional emit. Every finish path routes + // through here so a request's bookkeeping is torn down exactly once, in exactly one place. + // The trailing bool knobs preserve pre-existing per-callsite asymmetries (OnPush's dispatch + // catch never tracked ResetBackpressure; DispatchAsync's synchronous branches never touched + // the in-flight metric at all) rather than silently changing behavior during the merge. + private void FinishRequest( + int seq, + IFeatureCollection features, + Exception? error, + int? failStatus, + bool emit, + bool cleanupSlot = true, + bool trackMetrics = true, + bool resetBackpressure = true, + bool timedOut = false) + { + if (failStatus is int status) + { + var responseFeature = features.Get(); + responseFeature?.StatusCode = status; + } + + CompleteResponseBody(features); + FireOnCompleted(features); + _inFlight--; + if (trackMetrics && _metricsEnabled) + { + if (timedOut) + { + Metrics.HandlerTimeouts().Add(1); + } + + Metrics.PipelineInFlight().Add(-1); + if (resetBackpressure) + { + ResetBackpressure(); + } + } + + if (cleanupSlot && _requestSlots.Remove(seq, out var slot)) + { + CleanupTimeout(slot); + DisposeAppContext(slot, error); + } + + if (emit) + { + Emit(features); + } + } + + private void TryCompleteIfDrained() + { + if (_upstreamFinished && _inFlight == 0) + { + CompleteStage(); + } + } + // ASP.NET's HostingApplication.CreateContext reuses its cached host context (and the // DefaultHttpContext graph) only when the collection it is handed implements // IHostContextContainer. The pooled GaudiFeatureCollection is non-generic, so we wrap @@ -485,34 +432,35 @@ private IFeatureCollection ContainerFor(IFeatureCollection features) return container; } - private void DisposeAppContext(int seq, Exception? exception) + private void DisposeAppContext(RequestSlot slot, Exception? exception) { - if (_appContexts.TryGetValue(seq, out var appCtx)) + if (slot.AppContext is { } appContext) { - _stage._application.DisposeContext(appCtx, exception); - _appContexts.Remove(seq); + _stage._application.DisposeContext(appContext, exception); } } private void CancelAllInFlight() { - foreach (var (_, cts) in _activeTimeouts) + foreach (var slot in _requestSlots.Values) { - cts.Cancel(); + slot.Cts?.Cancel(); } } - private void CleanupTimeout(int seq) + private void CleanupTimeout(RequestSlot slot) { - if (_timerKeys.Remove(seq, out var timerKeys)) + if (slot.SoftTimerKey is not null) + { + CancelTimer(slot.SoftTimerKey); + } + + if (slot.HardTimerKey is not null) { - CancelTimer(timerKeys.Soft); - CancelTimer(timerKeys.Hard); + CancelTimer(slot.HardTimerKey); } - _gracePhase.Remove(seq); - _activeFeatures.Remove(seq); - if (_activeTimeouts.Remove(seq, out var cts) && (!cts.TryReset() || !_ctsPool.TryReturn(cts))) + if (slot.Cts is { } cts && (!cts.TryReset() || !_ctsPool.TryReturn(cts))) { cts.Dispose(); } @@ -595,20 +543,26 @@ public override void PostStop() { Tracing.For("Handler").Info(this, "bridge PostStop (upstreamFinished={0}, inFlight={1})", _upstreamFinished, _inFlight); - foreach (var (_, features) in _activeFeatures) + + foreach (var slot in _requestSlots.Values) { - if (features.Get() is GaudiHttpRequestLifetimeFeature lifetime) + if (slot.Cts is not null) { - lifetime.Abort(); - } + if (slot.Features.Get() is GaudiHttpRequestLifetimeFeature lifetime) + { + lifetime.Abort(); + } - CompleteResponseBody(features); - } + CompleteResponseBody(slot.Features); - foreach (var (_, cts) in _activeTimeouts) - { - cts.Cancel(); - cts.Dispose(); + slot.Cts.Cancel(); + slot.Cts.Dispose(); + } + + if (slot.AppContext is { } appContext) + { + _stage._application.DisposeContext(appContext, null); + } } while (_ctsPool.TryRent(out var pooledCts)) @@ -616,15 +570,7 @@ public override void PostStop() pooledCts.Dispose(); } - foreach (var (_, appCtx) in _appContexts) - { - _stage._application.DisposeContext(appCtx, null); - } - - _activeFeatures.Clear(); - _activeTimeouts.Clear(); - _appContexts.Clear(); - _timerKeys.Clear(); + _requestSlots.Clear(); } } } diff --git a/src/GaudiHTTP/Streams/Stages/Server/HttpServerConnectionStageLogic.cs b/src/GaudiHTTP/Streams/Stages/Server/HttpServerConnectionStageLogic.cs index 9dcb0376..ae21d77b 100644 --- a/src/GaudiHTTP/Streams/Stages/Server/HttpServerConnectionStageLogic.cs +++ b/src/GaudiHTTP/Streams/Stages/Server/HttpServerConnectionStageLogic.cs @@ -87,6 +87,12 @@ public HttpServerConnectionStageLogic( return; } + // Deliberately not routed through TryPullNetwork(): unlike the other two call + // sites, this pull is triggered by handler demand for the *next* request, not by + // a signal that just changed ShouldPauseNetwork. Adding that guard here would + // change behavior for multiplexed protocols (H2/H3), where ShouldPauseNetwork can + // be true for an unrelated stream's body backpressure while this stream's queue is + // merely empty — gating on it would also stall new-request intake network-wide. if (!HasBeenPulled(_inNetwork) && !IsClosed(_inNetwork)) { Pull(_inNetwork); @@ -119,23 +125,18 @@ public HttpServerConnectionStageLogic( TryPushOutbound(); - if (_sm.ShouldComplete) + if (_metricsEnabled) { - if (_metricsEnabled) - { - OnResponseInstrumented(response); - } + OnResponseInstrumented(response); + } + if (_sm.ShouldComplete) + { Tracing.For(TraceCategory).Debug(this, "completing after response (connection close)"); CompleteAfterFlushingOutbound(); return; } - if (_metricsEnabled) - { - OnResponseInstrumented(response); - } - var bodyFeature = response.Get(); var hasBody = bodyFeature is not null; if (!hasBody) @@ -181,10 +182,7 @@ private void OnStageActorMessage((IActorRef sender, object message) args) { Tracing.For(TraceCategory).Trace(this, "body resumed"); _sm.ResumeBody(); - if (!_sm.ShouldPauseNetwork && !HasBeenPulled(_inNetwork) && !IsClosed(_inNetwork)) - { - Pull(_inNetwork); - } + TryPullNetwork(); return; } @@ -262,10 +260,7 @@ private void OnNetworkPush() TryPushRequest(); } - if (!_sm.ShouldPauseNetwork && !HasBeenPulled(_inNetwork) && !IsClosed(_inNetwork)) - { - Pull(_inNetwork); - } + TryPullNetwork(); TryPullResponse(); } @@ -400,6 +395,17 @@ private void TryPullResponse() } } + private bool TryPullNetwork() + { + if (!_sm.ShouldPauseNetwork && !HasBeenPulled(_inNetwork) && !IsClosed(_inNetwork)) + { + Pull(_inNetwork); + return true; + } + + return false; + } + [MethodImpl(MethodImplOptions.NoInlining)] private static void OnRequestInstrumented(IFeatureCollection features) {