HTTP/3: Fix flakey pooling test (#35168)

JamesNK · web-flow · commit fd65bd8a8288 · 2021-08-10T09:40:38.000+12:00
diff --git a/src/Servers/Kestrel/shared/test/Http3/Http3InMemory.cs b/src/Servers/Kestrel/shared/test/Http3/Http3InMemory.cs
@@ -200,6 +200,8 @@ internal void VerifyGoAway(Http3FrameWithPayload frame, long expectedLastStreamI
 
         public void AdvanceClock(TimeSpan timeSpan)
         {
+            Logger.LogDebug($"Advancing clock {timeSpan}.");
+
             var clock = _mockSystemClock;
             var endTime = clock.UtcNow + timeSpan;
 
@@ -221,10 +223,13 @@ public void TriggerTick(DateTimeOffset now)
 
         public async Task InitializeConnectionAsync(RequestDelegate application)
         {
-            MultiplexedConnectionContext = new TestMultiplexedConnectionContext(this);
+            MultiplexedConnectionContext = new TestMultiplexedConnectionContext(this)
+            {
+                ConnectionId = "TEST"
+            };
 
             var httpConnectionContext = new HttpMultiplexedConnectionContext(
-                connectionId: "TestConnectionId",
+                connectionId: MultiplexedConnectionContext.ConnectionId,
                 HttpProtocols.Http3,
                 altSvcHeader: null,
                 connectionContext: MultiplexedConnectionContext,
@@ -398,11 +403,16 @@ internal async ValueTask<Http3ControlStream> CreateControlStream(int? id)
 
         internal ValueTask<Http3RequestStream> CreateRequestStream(Http3RequestHeaderHandler headerHandler = null)
         {
+            var requestStreamId = GetStreamId(0x00);
             if (!_streamContextPool.TryDequeue(out var testStreamContext))
             {
                 testStreamContext = new TestStreamContext(canRead: true, canWrite: true, this);
             }
-            testStreamContext.Initialize(GetStreamId(0x00));
+            else
+            {
+                Logger.LogDebug($"Reusing context for request stream {requestStreamId}.");
+            }
+            testStreamContext.Initialize(requestStreamId);
 
             var stream = new Http3RequestStream(this, Connection, testStreamContext, headerHandler ?? new Http3RequestHeaderHandler());
             _runningStreams[stream.StreamId] = stream;
@@ -1102,22 +1112,29 @@ public override void Abort(ConnectionAbortedException abortReason)
 
         public override ValueTask DisposeAsync()
         {
-            _testBase.Logger.LogInformation($"Disposing stream {StreamId}");
-
-            Disposed = true;
-            _disposedTcs.TrySetResult();
+            _testBase.Logger.LogDebug($"Disposing stream {StreamId}");
 
+            var readerCompletedSuccessfully = _transportPipeReader.IsCompletedSuccessfully;
+            var writerCompletedSuccessfully = _transportPipeWriter.IsCompletedSuccessfully;
             var canReuse = !_isAborted &&
-                _transportPipeReader.IsCompletedSuccessfully &&
-                _transportPipeWriter.IsCompletedSuccessfully;
+                readerCompletedSuccessfully &&
+                writerCompletedSuccessfully;
 
             _pair.Transport.Input.Complete();
             _pair.Transport.Output.Complete();
 
             if (canReuse)
             {
+                _testBase.Logger.LogDebug($"Pooling stream {StreamId} for reuse.");
                 _testBase._streamContextPool.Enqueue(this);
             }
+            else
+            {
+                _testBase.Logger.LogDebug($"Can't reuse stream {StreamId}. Aborted: {_isAborted}, Reader completed successfully: {readerCompletedSuccessfully}, Writer completed successfully: {writerCompletedSuccessfully}.");
+            }
+
+            Disposed = true;
+            _disposedTcs.TrySetResult();
 
             return ValueTask.CompletedTask;
         }
diff --git a/src/Servers/Kestrel/shared/test/TestContextFactory.cs b/src/Servers/Kestrel/shared/test/TestContextFactory.cs
@@ -84,10 +84,10 @@ public static HttpMultiplexedConnectionContext CreateHttp3ConnectionContext(
             ITimeoutControl timeoutControl = null)
         {
             var http3ConnectionContext = new HttpMultiplexedConnectionContext(
-                "TestConnectionId",
+                "TEST",
                 HttpProtocols.Http3,
                 altSvcHeader: null,
-                connectionContext ?? new TestMultiplexedConnectionContext(),
+                connectionContext ?? new TestMultiplexedConnectionContext { ConnectionId = "TEST" },
                 serviceContext ?? CreateServiceContext(new KestrelServerOptions()),
                 connectionFeatures ?? new FeatureCollection(),
                 memoryPool ?? PinnedBlockMemoryPoolFactory.Create(),
diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3ConnectionTests.cs
@@ -12,6 +12,7 @@
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3;
 using Microsoft.AspNetCore.Testing;
+using Microsoft.Extensions.Logging;
 using Microsoft.Net.Http.Headers;
 using Xunit;
 using Http3SettingType = Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3.Http3SettingType;
@@ -90,6 +91,8 @@ public async Task GOAWAY_GracefulServerShutdown_SendsGoAway(int connectionReques
                 await request.SendHeadersAsync(Headers);
                 await request.EndStreamAsync();
                 await request.ExpectReceiveEndOfStream();
+
+                await request.OnStreamCompletedTask.DefaultTimeout();
             }
 
             // Trigger server shutdown.
@@ -272,16 +275,16 @@ public async Task StreamPool_MultipleStreamsInSequence_PooledStreamReused()
 
             await Http3Api.InitializeConnectionAsync(_echoApplication);
 
-            var streamContext1 = await MakeRequestAsync(0, headers);
-            var streamContext2 = await MakeRequestAsync(1, headers);
+            var streamContext1 = await MakeRequestAsync(0, headers, sendData: true, waitForServerDispose: true);
+            var streamContext2 = await MakeRequestAsync(1, headers, sendData: true, waitForServerDispose: true);
 
             Assert.Same(streamContext1, streamContext2);
         }
 
         [Theory]
         [InlineData(10)]
         [InlineData(100)]
-        [InlineData(1000)]
+        [InlineData(500)]
         [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/34685")]
         public async Task StreamPool_VariableMultipleStreamsInSequence_PooledStreamReused(int count)
         {
@@ -299,40 +302,82 @@ public async Task StreamPool_VariableMultipleStreamsInSequence_PooledStreamReuse
             ConnectionContext last = null;
             for (var i = 0; i < count; i++)
             {
-                var streamContext = await MakeRequestAsync(i, headers);
+                Logger.LogInformation($"Iteration {i}");
+
+                var streamContext = await MakeRequestAsync(i, headers, sendData: true, waitForServerDispose: true);
 
                 first ??= streamContext;
                 last = streamContext;
+
+                Assert.Same(first, last);
             }
+        }
+
+        [Theory]
+        [InlineData(10, false)]
+        [InlineData(10, true)]
+        [InlineData(100, false)]
+        [InlineData(100, true)]
+        [InlineData(500, false)]
+        [InlineData(500, true)]
+        [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/34685")]
+        public async Task VariableMultipleStreamsInSequence_Success(int count, bool sendData)
+        {
+            var headers = new[]
+            {
+                new KeyValuePair<string, string>(HeaderNames.Method, "Custom"),
+                new KeyValuePair<string, string>(HeaderNames.Path, "/"),
+                new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
+                new KeyValuePair<string, string>(HeaderNames.Authority, "localhost:80"),
+            };
+
+            var requestDelegate = sendData ? _echoApplication : _noopApplication;
 
-            Assert.Same(first, last);
+            await Http3Api.InitializeConnectionAsync(requestDelegate);
+
+            for (var i = 0; i < count; i++)
+            {
+                Logger.LogInformation($"Iteration {i}");
+
+                await MakeRequestAsync(i, headers, sendData, waitForServerDispose: false);
+            }
         }
 
-        private async Task<ConnectionContext> MakeRequestAsync(int index, KeyValuePair<string, string>[] headers)
+        private async Task<ConnectionContext> MakeRequestAsync(int index, KeyValuePair<string, string>[] headers, bool sendData, bool waitForServerDispose)
         {
             var requestStream = await Http3Api.CreateRequestStream();
             var streamContext = requestStream.StreamContext;
 
-            await requestStream.SendHeadersAsync(headers);
+            await requestStream.SendHeadersAsync(headers, endStream: !sendData);
 
-            await requestStream.SendDataAsync(Encoding.ASCII.GetBytes($"Hello world {index}"));
+            if (sendData)
+            {
+                await requestStream.SendDataAsync(Encoding.ASCII.GetBytes($"Hello world {index}"));
+            }
 
             await requestStream.ExpectHeadersAsync();
-            var responseData = await requestStream.ExpectDataAsync();
-            Assert.Equal($"Hello world {index}", Encoding.ASCII.GetString(responseData.ToArray()));
 
-            Assert.False(requestStream.Disposed, "Request is in progress and shouldn't be disposed.");
+            if (sendData)
+            {
+                var responseData = await requestStream.ExpectDataAsync();
+                Assert.Equal($"Hello world {index}", Encoding.ASCII.GetString(responseData.ToArray()));
+
+                Assert.False(requestStream.Disposed, "Request is in progress and shouldn't be disposed.");
 
-            await requestStream.SendDataAsync(Encoding.ASCII.GetBytes($"End {index}"), endStream: true);
-            responseData = await requestStream.ExpectDataAsync();
-            Assert.Equal($"End {index}", Encoding.ASCII.GetString(responseData.ToArray()));
+                await requestStream.SendDataAsync(Encoding.ASCII.GetBytes($"End {index}"), endStream: true);
+                responseData = await requestStream.ExpectDataAsync();
+                Assert.Equal($"End {index}", Encoding.ASCII.GetString(responseData.ToArray()));
+            }
 
             await requestStream.ExpectReceiveEndOfStream();
 
-            await requestStream.OnStreamCompletedTask.DefaultTimeout();
+            if (waitForServerDispose)
+            {
+                await requestStream.OnDisposedTask.DefaultTimeout();
+                Assert.True(requestStream.Disposed, "Request is complete and should be disposed.");
 
-            await requestStream.OnDisposedTask.DefaultTimeout();
-            Assert.True(requestStream.Disposed, "Request is complete and should be disposed.");
+                Logger.LogInformation($"Received notification that stream {index} disposed.");
+            }
 
             return streamContext;
         }
diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3TimeoutTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3TimeoutTests.cs
@@ -12,6 +12,7 @@
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
 using Microsoft.AspNetCore.Testing;
+using Microsoft.Extensions.Logging;
 using Microsoft.Net.Http.Headers;
 using Moq;
 using Xunit;
@@ -502,6 +503,7 @@ public async Task DATA_Received_TooSlowlyOnSecondStream_AbortsConnectionAfterNon
             var inboundControlStream = await Http3Api.GetInboundControlStream();
             await inboundControlStream.ExpectSettingsAsync();
 
+            Logger.LogInformation("Sending first request");
             var requestStream1 = await Http3Api.CreateRequestStream();
 
             // _maxData is 16 KiB, and 16 KiB / 240 bytes/sec ~= 68 secs which is far above the grace period.
@@ -513,6 +515,7 @@ public async Task DATA_Received_TooSlowlyOnSecondStream_AbortsConnectionAfterNon
 
             await requestStream1.ExpectReceiveEndOfStream();
 
+            Logger.LogInformation("Sending second request");
             var requestStream2 = await Http3Api.CreateRequestStream();
 
             await requestStream2.SendHeadersAsync(ReadRateRequestHeaders(_maxData.Length), endStream: false);
