Skip to content

Commit 07001a3

Browse files
JamesNKJamesNK
authored
HTTP/3: Fix flakey request headers timeout tests (#32131)
1 parent 4167ded commit 07001a3

File tree

6 files changed

+133
-62
lines changed

6 files changed

+133
-62
lines changed

‎src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs‎

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
2323
{
2424
internal class Http3Connection : ITimeoutHandler, IHttp3StreamLifetimeHandler
2525
{
26+
// Internal for unit testing
2627
internal readonly Dictionary<long, IHttp3Stream> _streams = new Dictionary<long, IHttp3Stream>();
28+
internal IHttp3StreamLifetimeHandler _streamLifetimeHandler;
2729

2830
private long _highestOpenedStreamId;
2931
private readonly object _sync = new object();
@@ -49,6 +51,7 @@ public Http3Connection(Http3ConnectionContext context)
4951
_systemClock = context.ServiceContext.SystemClock;
5052
_timeoutControl = new TimeoutControl(this);
5153
_context.TimeoutControl ??= _timeoutControl;
54+
_streamLifetimeHandler = this;
5255

5356
_errorCodeFeature = context.ConnectionFeatures.Get<IProtocolErrorCodeFeature>()!;
5457

@@ -303,7 +306,7 @@ internal async Task InnerProcessStreamsAsync<TContext>(IHttpApplication<TContext
303306
streamContext.LocalEndPoint as IPEndPoint,
304307
streamContext.RemoteEndPoint as IPEndPoint,
305308
streamContext.Transport,
306-
this,
309+
_streamLifetimeHandler,
307310
streamContext,
308311
_serverSettings);
309312
httpConnectionContext.TimeoutControl = _context.TimeoutControl;
@@ -314,10 +317,7 @@ internal async Task InnerProcessStreamsAsync<TContext>(IHttpApplication<TContext
314317
{
315318
// Unidirectional stream
316319
var stream = new Http3ControlStream<TContext>(application, httpConnectionContext);
317-
lock (_streams)
318-
{
319-
_streams[streamId] = stream;
320-
}
320+
_streamLifetimeHandler.OnStreamCreated(stream);
321321

322322
ThreadPool.UnsafeQueueUserWorkItem(stream, preferLocal: false);
323323
}
@@ -327,11 +327,7 @@ internal async Task InnerProcessStreamsAsync<TContext>(IHttpApplication<TContext
327327
UpdateHighestStreamId(streamId);
328328

329329
var stream = new Http3Stream<TContext>(application, httpConnectionContext);
330-
lock (_streams)
331-
{
332-
_activeRequestCount++;
333-
_streams[streamId] = stream;
334-
}
330+
_streamLifetimeHandler.OnStreamCreated(stream);
335331

336332
KestrelEventSource.Log.RequestQueuedStart(stream, AspNetCore.Http.HttpProtocol.Http3);
337333
ThreadPool.UnsafeQueueUserWorkItem(stream, preferLocal: false);
@@ -470,7 +466,7 @@ private async ValueTask<Http3ControlStream> CreateNewUnidirectionalStreamAsync<T
470466
streamContext.LocalEndPoint as IPEndPoint,
471467
streamContext.RemoteEndPoint as IPEndPoint,
472468
streamContext.Transport,
473-
this,
469+
_streamLifetimeHandler,
474470
streamContext,
475471
_serverSettings);
476472
httpConnectionContext.TimeoutControl = _context.TimeoutControl;
@@ -490,7 +486,7 @@ private ValueTask<FlushResult> SendGoAway(long id)
490486
return default;
491487
}
492488

493-
public bool OnInboundControlStream(Http3ControlStream stream)
489+
bool IHttp3StreamLifetimeHandler.OnInboundControlStream(Http3ControlStream stream)
494490
{
495491
lock (_sync)
496492
{
@@ -503,7 +499,7 @@ public bool OnInboundControlStream(Http3ControlStream stream)
503499
}
504500
}
505501

506-
public bool OnInboundEncoderStream(Http3ControlStream stream)
502+
bool IHttp3StreamLifetimeHandler.OnInboundEncoderStream(Http3ControlStream stream)
507503
{
508504
lock (_sync)
509505
{
@@ -516,7 +512,7 @@ public bool OnInboundEncoderStream(Http3ControlStream stream)
516512
}
517513
}
518514

519-
public bool OnInboundDecoderStream(Http3ControlStream stream)
515+
bool IHttp3StreamLifetimeHandler.OnInboundDecoderStream(Http3ControlStream stream)
520516
{
521517
lock (_sync)
522518
{
@@ -529,24 +525,42 @@ public bool OnInboundDecoderStream(Http3ControlStream stream)
529525
}
530526
}
531527

532-
public void OnStreamCompleted(IHttp3Stream stream)
528+
void IHttp3StreamLifetimeHandler.OnStreamCreated(IHttp3Stream stream)
533529
{
534530
lock (_streams)
535531
{
536-
_activeRequestCount--;
532+
if (stream.IsRequestStream)
533+
{
534+
_activeRequestCount++;
535+
}
536+
_streams[stream.StreamId] = stream;
537+
}
538+
}
539+
540+
void IHttp3StreamLifetimeHandler.OnStreamCompleted(IHttp3Stream stream)
541+
{
542+
lock (_streams)
543+
{
544+
if (stream.IsRequestStream)
545+
{
546+
_activeRequestCount--;
547+
}
537548
_streams.Remove(stream.StreamId);
538549
}
539550

540-
_streamCompletionAwaitable.Complete();
551+
if (stream.IsRequestStream)
552+
{
553+
_streamCompletionAwaitable.Complete();
554+
}
541555
}
542556

543-
public void OnStreamConnectionError(Http3ConnectionErrorException ex)
557+
void IHttp3StreamLifetimeHandler.OnStreamConnectionError(Http3ConnectionErrorException ex)
544558
{
545559
Log.Http3ConnectionError(ConnectionId, ex);
546560
Abort(new ConnectionAbortedException(ex.Message, ex), ex.ErrorCode);
547561
}
548562

549-
public void OnInboundControlStreamSetting(Http3SettingType type, long value)
563+
void IHttp3StreamLifetimeHandler.OnInboundControlStreamSetting(Http3SettingType type, long value)
550564
{
551565
switch (type)
552566
{
@@ -561,6 +575,11 @@ public void OnInboundControlStreamSetting(Http3SettingType type, long value)
561575
}
562576
}
563577

578+
void IHttp3StreamLifetimeHandler.OnStreamHeaderReceived(IHttp3Stream stream)
579+
{
580+
Debug.Assert(stream.ReceivedHeader);
581+
}
582+
564583
private static class GracefulCloseInitiator
565584
{
566585
public const int None = 0;

‎src/Servers/Kestrel/Core/src/Internal/Http3/Http3ControlStream.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ public async Task ProcessRequestAsync<TContext>(IHttpApplication<TContext> appli
154154
try
155155
{
156156
_headerType = await TryReadStreamHeaderAsync();
157+
_context.StreamLifetimeHandler.OnStreamHeaderReceived(this);
157158

158159
switch (_headerType)
159160
{
@@ -195,6 +196,10 @@ public async Task ProcessRequestAsync<TContext>(IHttpApplication<TContext> appli
195196
_errorCodeFeature.Error = (long)ex.ErrorCode;
196197
_context.StreamLifetimeHandler.OnStreamConnectionError(ex);
197198
}
199+
finally
200+
{
201+
_context.StreamLifetimeHandler.OnStreamCompleted(this);
202+
}
198203
}
199204

200205
private async Task HandleControlStream()

‎src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,7 @@ private async Task ProcessHeadersFrameAsync<TContext>(IHttpApplication<TContext>
549549
}
550550

551551
_appCompleted = new TaskCompletionSource();
552+
_context.StreamLifetimeHandler.OnStreamHeaderReceived(this);
552553

553554
ThreadPool.UnsafeQueueUserWorkItem(this, preferLocal: false);
554555
}

‎src/Servers/Kestrel/Core/src/Internal/Http3/IHttp3StreamLifetimeHandler.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
55
{
66
internal interface IHttp3StreamLifetimeHandler
77
{
8+
void OnStreamCreated(IHttp3Stream stream);
9+
void OnStreamHeaderReceived(IHttp3Stream stream);
810
void OnStreamCompleted(IHttp3Stream stream);
911
void OnStreamConnectionError(Http3ConnectionErrorException ex);
1012

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3TestBase.cs‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Buffers;
6+
using System.Collections.Concurrent;
67
using System.Collections.Generic;
78
using System.Diagnostics;
89
using System.IO;
@@ -47,6 +48,7 @@ public abstract class Http3TestBase : TestApplicationErrorLoggerLoggedTest, IDis
4748
protected Task _connectionTask;
4849
protected readonly TaskCompletionSource _closedStateReached = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
4950

51+
internal readonly ConcurrentDictionary<long, Http3StreamBase> _runningStreams = new ConcurrentDictionary<long, Http3StreamBase>();
5052

5153
protected readonly RequestDelegate _noopApplication;
5254
protected readonly RequestDelegate _echoApplication;
@@ -242,10 +244,74 @@ protected void CreateConnection()
242244
httpConnectionContext.TimeoutControl = _mockTimeoutControl.Object;
243245

244246
Connection = new Http3Connection(httpConnectionContext);
247+
Connection._streamLifetimeHandler = new LifetimeHandlerInterceptor(Connection, this);
248+
245249
_mockTimeoutHandler.Setup(h => h.OnTimeout(It.IsAny<TimeoutReason>()))
246250
.Callback<TimeoutReason>(r => Connection.OnTimeout(r));
247251
}
248252

253+
private class LifetimeHandlerInterceptor : IHttp3StreamLifetimeHandler
254+
{
255+
private readonly IHttp3StreamLifetimeHandler _inner;
256+
private readonly Http3TestBase _http3TestBase;
257+
258+
public LifetimeHandlerInterceptor(IHttp3StreamLifetimeHandler inner, Http3TestBase http3TestBase)
259+
{
260+
_inner = inner;
261+
_http3TestBase = http3TestBase;
262+
}
263+
264+
public bool OnInboundControlStream(Internal.Http3.Http3ControlStream stream)
265+
{
266+
return _inner.OnInboundControlStream(stream);
267+
}
268+
269+
public void OnInboundControlStreamSetting(Internal.Http3.Http3SettingType type, long value)
270+
{
271+
_inner.OnInboundControlStreamSetting(type, value);
272+
}
273+
274+
public bool OnInboundDecoderStream(Internal.Http3.Http3ControlStream stream)
275+
{
276+
return _inner.OnInboundDecoderStream(stream);
277+
}
278+
279+
public bool OnInboundEncoderStream(Internal.Http3.Http3ControlStream stream)
280+
{
281+
return _inner.OnInboundEncoderStream(stream);
282+
}
283+
284+
public void OnStreamCompleted(IHttp3Stream stream)
285+
{
286+
_inner.OnStreamCompleted(stream);
287+
}
288+
289+
public void OnStreamConnectionError(Http3ConnectionErrorException ex)
290+
{
291+
_inner.OnStreamConnectionError(ex);
292+
}
293+
294+
public void OnStreamCreated(IHttp3Stream stream)
295+
{
296+
_inner.OnStreamCreated(stream);
297+
298+
if (_http3TestBase._runningStreams.TryGetValue(stream.StreamId, out var testStream))
299+
{
300+
testStream._onStreamCreatedTcs.TrySetResult();
301+
}
302+
}
303+
304+
public void OnStreamHeaderReceived(IHttp3Stream stream)
305+
{
306+
_inner.OnStreamHeaderReceived(stream);
307+
308+
if (_http3TestBase._runningStreams.TryGetValue(stream.StreamId, out var testStream))
309+
{
310+
testStream._onHeaderReceivedTcs.TrySetResult();
311+
}
312+
}
313+
}
314+
249315
protected void ConnectionClosed()
250316
{
251317

@@ -294,6 +360,8 @@ public ValueTask<Http3ControlStream> CreateControlStream()
294360
public async ValueTask<Http3ControlStream> CreateControlStream(int? id)
295361
{
296362
var stream = new Http3ControlStream(this, StreamInitiator.Client);
363+
_runningStreams[stream.StreamId] = stream;
364+
297365
MultiplexedConnectionContext.ToServerAcceptQueue.Writer.TryWrite(stream.StreamContext);
298366
if (id != null)
299367
{
@@ -305,6 +373,8 @@ public async ValueTask<Http3ControlStream> CreateControlStream(int? id)
305373
internal ValueTask<Http3RequestStream> CreateRequestStream()
306374
{
307375
var stream = new Http3RequestStream(this, Connection);
376+
_runningStreams[stream.StreamId] = stream;
377+
308378
MultiplexedConnectionContext.ToServerAcceptQueue.Writer.TryWrite(stream.StreamContext);
309379
return new ValueTask<Http3RequestStream>(stream);
310380
}
@@ -318,12 +388,18 @@ public ValueTask<ConnectionContext> StartBidirectionalStreamAsync()
318388

319389
public class Http3StreamBase : IProtocolErrorCodeFeature
320390
{
391+
internal TaskCompletionSource _onStreamCreatedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
392+
internal TaskCompletionSource _onHeaderReceivedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
393+
321394
internal DuplexPipe.DuplexPipePair _pair;
322395
internal Http3TestBase _testBase;
323396
internal Http3Connection _connection;
324397
private long _bytesReceived;
325398
public long Error { get; set; }
326399

400+
public Task OnStreamCreatedTask => _onStreamCreatedTcs.Task;
401+
public Task OnHeaderReceivedTask => _onHeaderReceivedTcs.Task;
402+
327403
protected Task SendAsync(ReadOnlySpan<byte> span)
328404
{
329405
var writableBuffer = _pair.Application.Output;

0 commit comments

Comments
 (0)