Improve RequestAborted token handling (#2977)

- Improve test reliability of tests verifying the RequestAborted token gets tripped
- Once the response body is completed, don't fire the token for that request even if it is accessed later on.

src/Kestrel.Core/Internal/ConnectionDispatcher.cs

@@ -82,12 +82,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
             }
             finally
             {
+                Log.ConnectionStop(connectionContext.ConnectionId);
+                KestrelEventSource.Log.ConnectionStop(connectionContext);
+
                 connection.Complete();
 
                 _serviceContext.ConnectionManager.RemoveConnection(id);
-
-                Log.ConnectionStop(connectionContext.ConnectionId);
-                KestrelEventSource.Log.ConnectionStop(connectionContext);
             }
         }
 

src/Kestrel.Core/Internal/Http/HttpProtocol.cs

@@ -39,12 +39,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
         private Stack<KeyValuePair<Func<object, Task>, object>> _onStarting;
         private Stack<KeyValuePair<Func<object, Task>, object>> _onCompleted;
 
-        private volatile int _ioCompleted;
+        private object _abortLock = new object();
+        private volatile bool _requestAborted;
+        private bool _preventRequestAbortedCancellation;
         private CancellationTokenSource _abortedCts;
         private CancellationToken? _manuallySetRequestAbortToken;
 
         protected RequestProcessingStatus _requestProcessingStatus;
-        protected volatile bool _keepAlive; // volatile, see: https://msdn.microsoft.com/en-us/library/x13ttww7.aspx
+
+        // Keep-alive is default for HTTP/1.1 and HTTP/2; parsing and errors will change its value
+        // volatile, see: https://msdn.microsoft.com/en-us/library/x13ttww7.aspx
+        protected volatile bool _keepAlive = true;
         protected bool _upgradeAvailable;
         private bool _canHaveBody;
         private bool _autoChunk;
@@ -235,16 +240,26 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 {
                     return _manuallySetRequestAbortToken.Value;
                 }
-                // Otherwise, get the abort CTS.  If we have one, which would mean that someone previously
+
-                // asked for the RequestAborted token, simply return its token.  If we don't,
+                lock (_abortLock)
-                // check to see whether we've already aborted, in which case just return an
+                {
-                // already canceled token.  Finally, force a source into existence if we still
+                    if (_preventRequestAbortedCancellation)
-                // don't have one, and return its token.
+                    {
-                var cts = _abortedCts;
+                        return new CancellationToken(false);
-                return
+                    }
-                    cts != null ? cts.Token :
+
-                    (_ioCompleted == 1) ? new CancellationToken(true) :
+                    if (_requestAborted)
-                    RequestAbortedSource.Token;
+                    {
+                        return new CancellationToken(true);
+                    }
+
+                    if (_abortedCts == null)
+                    {
+                        _abortedCts = new CancellationTokenSource();
+                    }
+
+                    return _abortedCts.Token;
+                }
             }
             set
             {
@@ -254,28 +269,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             }
         }
 
-        private CancellationTokenSource RequestAbortedSource
-        {
-            get
-            {
-                // Get the abort token, lazily-initializing it if necessary.
-                // Make sure it's canceled if an abort request already came in.
-
-                // EnsureInitialized can return null since _abortedCts is reset to null
-                // after it's already been initialized to a non-null value.
-                // If EnsureInitialized does return null, this property was accessed between
-                // requests so it's safe to return an ephemeral CancellationTokenSource.
-                var cts = LazyInitializer.EnsureInitialized(ref _abortedCts, () => new CancellationTokenSource())
-                            ?? new CancellationTokenSource();
-
-                if (_ioCompleted == 1)
-                {
-                    cts.Cancel();
-                }
-                return cts;
-            }
-        }
-
         public bool HasResponseStarted => _requestProcessingStatus == RequestProcessingStatus.ResponseStarted;
 
         protected HttpRequestHeaders HttpRequestHeaders { get; } = new HttpRequestHeaders();
@@ -354,9 +347,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             Scheme = _scheme;
 
             _manuallySetRequestAbortToken = null;
-            _abortedCts = null;
+            _preventRequestAbortedCancellation = false;
+
+            // Lock to prevent CancelRequestAbortedToken from attempting to cancel an disposed CTS.
+            lock (_abortLock)
+            {
+                if (!_requestAborted)
+                {
+                    _abortedCts?.Dispose();
+                    _abortedCts = null;
+                }
+            }
 
-            // Allow two bytes for \r\n after headers
             _requestHeadersParsed = 0;
 
             _responseBytesWritten = 0;
@@ -401,7 +403,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
         {
             try
             {
-                RequestAbortedSource.Cancel();
+                _abortedCts.Cancel();
+                _abortedCts.Dispose();
                 _abortedCts = null;
             }
             catch (Exception ex)
@@ -412,15 +415,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 
         protected void AbortRequest()
         {
-            if (Interlocked.Exchange(ref _ioCompleted, 1) != 0)
+            lock (_abortLock)
             {
-                return;
+                if (_requestAborted)
+                {
+                    return;
+                }
+
+                _requestAborted = true;
             }
 
-            _keepAlive = false;
+            if (_abortedCts != null)
-
+            {
-            // Potentially calling user code. CancelRequestAbortedToken logs any exceptions.
+                // Potentially calling user code. CancelRequestAbortedToken logs any exceptions.
-            ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this);
+                ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this);
+            }
         }
 
         protected void PoisonRequestBodyStream(Exception abortReason)
@@ -428,6 +437,22 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             _streams?.Abort(abortReason);
         }
 
+        // Prevents the RequestAborted token from firing for the duration of the request.
+        private void PreventRequestAbortedCancellation()
+        {
+            lock (_abortLock)
+            {
+                if (_requestAborted)
+                {
+                    return;
+                }
+
+                _preventRequestAbortedCancellation = true;
+                _abortedCts?.Dispose();
+                _abortedCts = null;
+            }
+        }
+
         public void OnHeader(Span<byte> name, Span<byte> value)
         {
             _requestHeadersParsed++;
@@ -487,9 +512,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 
         private async Task ProcessRequests<TContext>(IHttpApplication<TContext> application)
         {
-            // Keep-alive is default for HTTP/1.1 and HTTP/2; parsing and errors will change its value
-            _keepAlive = true;
-
             while (_keepAlive)
             {
                 BeginRequestProcessing();
@@ -529,7 +551,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                     // Run the application code for this request
                     await application.ProcessRequestAsync(httpContext);
 
-                    if (_ioCompleted == 0)
+                    if (!_requestAborted)
                     {
                         VerifyResponseContentLength();
                     }
@@ -565,7 +587,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 // 4XX responses are written by TryProduceInvalidRequestResponse during connection tear down.
                 if (_requestRejectedException == null)
                 {
-                    if (_ioCompleted == 0)
+                    if (!_requestAborted)
                     {
                         // Call ProduceEnd() before consuming the rest of the request body to prevent
                         // delaying clients waiting for the chunk terminator:
@@ -597,7 +619,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 application.DisposeContext(httpContext, _applicationException);
 
                 // Even for non-keep-alive requests, try to consume the entire body to avoid RSTs.
-                if (_ioCompleted == 0 && _requestRejectedException == null && !messageBody.IsEmpty)
+                if (!_requestAborted && _requestRejectedException == null && !messageBody.IsEmpty)
                 {
                     await messageBody.ConsumeAsync();
                 }
@@ -877,7 +899,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 responseHeaders.ContentLength.HasValue &&
                 _responseBytesWritten == responseHeaders.ContentLength.Value)
             {
-                _abortedCts = null;
+                PreventRequestAbortedCancellation();
             }
         }
 
@@ -995,8 +1017,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 
         protected Task TryProduceInvalidRequestResponse()
         {
-            // If _ioCompleted is set, the connection has already been closed.
+            // If _requestAborted is set, the connection has already been closed.
-            if (_requestRejectedException != null && _ioCompleted == 0)
+            if (_requestRejectedException != null && !_requestAborted)
             {
                 return ProduceEnd();
             }
@@ -1073,7 +1095,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
         private async Task WriteSuffixAwaited()
         {
             // For the same reason we call CheckLastWrite() in Content-Length responses.
-            _abortedCts = null;
+            PreventRequestAbortedCancellation();
 
             await Output.WriteStreamSuffixAsync();
 

test/Kestrel.Core.Tests/Http1ConnectionTests.cs

@@ -640,17 +640,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
         {
             _http1Connection.ResponseHeaders["Content-Length"] = "12";
 
-            // Need to compare WaitHandle ref since CancellationToken is struct
+            var original = _http1Connection.RequestAborted;
-            var original = _http1Connection.RequestAborted.WaitHandle;
 
             foreach (var ch in "hello, worl")
             {
                 await _http1Connection.WriteAsync(new ArraySegment<byte>(new[] { (byte)ch }));
-                Assert.Same(original, _http1Connection.RequestAborted.WaitHandle);
+                Assert.Equal(original, _http1Connection.RequestAborted);
             }
 
             await _http1Connection.WriteAsync(new ArraySegment<byte>(new[] { (byte)'d' }));
-            Assert.NotSame(original, _http1Connection.RequestAborted.WaitHandle);
+            Assert.NotEqual(original, _http1Connection.RequestAborted);
+
+            _http1Connection.Abort(new ConnectionAbortedException());
+
+            Assert.False(original.IsCancellationRequested);
+            Assert.False(_http1Connection.RequestAborted.IsCancellationRequested);
         }
 
         [Fact]
@@ -658,17 +662,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
         {
             _http1Connection.ResponseHeaders["Content-Length"] = "12";
 
-            // Need to compare WaitHandle ref since CancellationToken is struct
+            var original = _http1Connection.RequestAborted;
-            var original = _http1Connection.RequestAborted.WaitHandle;
 
             foreach (var ch in "hello, worl")
             {
                 await _http1Connection.WriteAsync(new ArraySegment<byte>(new[] { (byte)ch }), default(CancellationToken));
-                Assert.Same(original, _http1Connection.RequestAborted.WaitHandle);
+                Assert.Equal(original, _http1Connection.RequestAborted);
             }
 
             await _http1Connection.WriteAsync(new ArraySegment<byte>(new[] { (byte)'d' }), default(CancellationToken));
-            Assert.NotSame(original, _http1Connection.RequestAborted.WaitHandle);
+            Assert.NotEqual(original, _http1Connection.RequestAborted);
+
+            _http1Connection.Abort(new ConnectionAbortedException());
+
+            Assert.False(original.IsCancellationRequested);
+            Assert.False(_http1Connection.RequestAborted.IsCancellationRequested);
         }
 
         [Fact]
@@ -676,36 +684,44 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
         {
             _http1Connection.ResponseHeaders["Content-Length"] = "12";
 
-            // Need to compare WaitHandle ref since CancellationToken is struct
+            var original = _http1Connection.RequestAborted;
-            var original = _http1Connection.RequestAborted.WaitHandle;
 
             // Only first write can be WriteAsyncAwaited
             var startingTask = _http1Connection.InitializeResponseAwaited(Task.CompletedTask, 1);
             await _http1Connection.WriteAsyncAwaited(startingTask, new ArraySegment<byte>(new[] { (byte)'h' }), default(CancellationToken));
-            Assert.Same(original, _http1Connection.RequestAborted.WaitHandle);
+            Assert.Equal(original, _http1Connection.RequestAborted);
 
             foreach (var ch in "ello, worl")
             {
                 await _http1Connection.WriteAsync(new ArraySegment<byte>(new[] { (byte)ch }), default(CancellationToken));
-                Assert.Same(original, _http1Connection.RequestAborted.WaitHandle);
+                Assert.Equal(original, _http1Connection.RequestAborted);
             }
 
             await _http1Connection.WriteAsync(new ArraySegment<byte>(new[] { (byte)'d' }), default(CancellationToken));
-            Assert.NotSame(original, _http1Connection.RequestAborted.WaitHandle);
+            Assert.NotEqual(original, _http1Connection.RequestAborted);
+
+            _http1Connection.Abort(new ConnectionAbortedException());
+
+            Assert.False(original.IsCancellationRequested);
+            Assert.False(_http1Connection.RequestAborted.IsCancellationRequested);
         }
 
         [Fact]
         public async Task RequestAbortedTokenIsResetBeforeLastWriteWithChunkedEncoding()
         {
-            // Need to compare WaitHandle ref since CancellationToken is struct
+            var original = _http1Connection.RequestAborted;
-            var original = _http1Connection.RequestAborted.WaitHandle;
 
             _http1Connection.HttpVersion = "HTTP/1.1";
             await _http1Connection.WriteAsync(new ArraySegment<byte>(Encoding.ASCII.GetBytes("hello, world")), default(CancellationToken));
-            Assert.Same(original, _http1Connection.RequestAborted.WaitHandle);
+            Assert.Equal(original, _http1Connection.RequestAborted);
 
             await _http1Connection.ProduceEndAsync();
-            Assert.NotSame(original, _http1Connection.RequestAborted.WaitHandle);
+            Assert.NotEqual(original, _http1Connection.RequestAborted);
+
+            _http1Connection.Abort(new ConnectionAbortedException());
+
+            Assert.False(original.IsCancellationRequested);
+            Assert.False(_http1Connection.RequestAborted.IsCancellationRequested);
         }
 
         [Fact]

test/Kestrel.InMemory.FunctionalTests/ConnectionLimitTests.cs

@@ -201,7 +201,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests
 
         private TestServer CreateServerWithMaxConnections(RequestDelegate app, ResourceCounter concurrentConnectionCounter)
         {
-            var serviceContext = new TestServiceContext(LoggerFactory);
+            var serviceContext = new TestServiceContext(LoggerFactory)
+            {
+                ExpectedConnectionMiddlewareCount = 1
+            };
+
             var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0));
             listenOptions.Use(next =>
             {

test/Kestrel.InMemory.FunctionalTests/TestTransport/TestServer.cs

@@ -14,6 +14,7 @@ using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Core;
 using Microsoft.AspNetCore.Testing;
 using Microsoft.Extensions.DependencyInjection;
+using Xunit;
 
 namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTransport
 {
@@ -66,7 +67,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTrans
             _transportFactory = new InMemoryTransportFactory();
             HttpClientSlim = new InMemoryHttpClientSlim(this);
 
-
             var hostBuilder = new WebHostBuilder()
                 .ConfigureServices(services =>
                 {
@@ -79,6 +79,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTrans
                     {
                         context.ServerOptions.ApplicationServices = sp;
                         configureKestrel(context.ServerOptions);
+
+                        // Prevent ListenOptions reuse. This is easily done accidentally when trying to debug a test by running it
+                        // in a loop, but will cause problems because only the app func from the first loop will ever be invoked.
+                        Assert.All(context.ServerOptions.ListenOptions, lo =>
+                            Assert.Equal(context.ExpectedConnectionMiddlewareCount, lo._middleware.Count));
+
                         return new KestrelServer(_transportFactory, context);
                     });
                 });

test/Kestrel.Transport.FunctionalTests/RequestTests.cs

@@ -679,6 +679,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                         readTcs.SetException(ex);
                         throw;
                     }
+                    finally
+                    {
+                        await registrationTcs.Task.DefaultTimeout();
+                    }
 
                     readTcs.SetException(new Exception("This shouldn't be reached."));
                 }
@@ -711,7 +715,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                 }
             }
 
-            await Assert.ThrowsAsync<TaskCanceledException>(async () => await readTcs.Task);
+            await Assert.ThrowsAsync<TaskCanceledException>(async () => await readTcs.Task).DefaultTimeout();
 
             // The cancellation token for only the last request should be triggered.
             var abortedRequestId = await registrationTcs.Task.DefaultTimeout();

test/Kestrel.Transport.FunctionalTests/ResponseTests.cs

@@ -211,6 +211,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                     writeTcs.SetException(ex);
                     throw;
                 }
+                finally
+                {
+                    await requestAbortedWh.Task.DefaultTimeout();
+                }
 
                 writeTcs.SetException(new Exception("This shouldn't be reached."));
             }, new TestServiceContext(LoggerFactory), listenOptions))
@@ -243,7 +247,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
             const int connectionPausedEventId = 4;
             const int maxRequestBufferSize = 4096;
 
-            var requestAborted = false;
+            var requestAborted = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
             var readCallbackUnwired = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
             var clientClosedConnection = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
             var writeTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
@@ -291,10 +295,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
 
             using (var server = new TestServer(async context =>
             {
-                context.RequestAborted.Register(() =>
+                context.RequestAborted.Register(() => requestAborted.SetResult(null));
-                {
-                    requestAborted = true;
-                });
 
                 await clientClosedConnection.Task;
 
@@ -311,6 +312,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                     writeTcs.SetException(ex);
                     throw;
                 }
+                finally
+                {
+                    await requestAborted.Task.DefaultTimeout();
+                }
 
                 writeTcs.SetException(new Exception("This shouldn't be reached."));
             }, testContext, listenOptions))
@@ -336,7 +341,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
             }
 
             mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny<string>()), Times.Once());
-            Assert.True(requestAborted);
+            Assert.True(requestAborted.Task.IsCompleted);
         }
 
         [Theory]
@@ -345,22 +350,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
         {
             const int connectionResetEventId = 19;
             const int connectionFinEventId = 6;
-            //const int connectionStopEventId = 2;
+            const int connectionStopEventId = 2;
 
             const int responseBodySegmentSize = 65536;
             const int responseBodySegmentCount = 100;
 
+            var requestAborted = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
             var appCompletedTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
-            var requestAborted = false;
 
             var scratchBuffer = new byte[responseBodySegmentSize];
 
             using (var server = new TestServer(async context =>
             {
-                context.RequestAborted.Register(() =>
+                context.RequestAborted.Register(() => requestAborted.SetResult(null));
-                {
-                    requestAborted = true;
-                });
 
                 for (var i = 0; i < responseBodySegmentCount; i++)
                 {
@@ -368,6 +370,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                     await Task.Delay(10);
                 }
 
+                await requestAborted.Task.DefaultTimeout();
                 appCompletedTcs.SetResult(null);
             }, new TestServiceContext(LoggerFactory), listenOptions))
             {
@@ -386,9 +389,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                     connection.Reset();
                 }
 
-                await appCompletedTcs.Task.DefaultTimeout();
+                await requestAborted.Task.DefaultTimeout();
 
-                // After the app is done with the write loop, the connection reset should be logged.
+                // After the RequestAborted token is tripped, the connection reset should be logged.
                 // On Linux and macOS, the connection close is still sometimes observed as a FIN despite the LingerState.
                 var presShutdownTransportLogs = TestSink.Writes.Where(
                     w => w.LoggerName == "Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv" ||
@@ -398,14 +401,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                          (!TestPlatformHelper.IsWindows && w.EventId == connectionFinEventId));
 
                 Assert.NotEmpty(connectionResetLogs);
+
+                // On macOS, the default 5 shutdown timeout is insufficient for the write loop to complete, so give it extra time.
+                await appCompletedTcs.Task.DefaultTimeout();
             }
 
-            // TODO: Figure out what the following assertion is flaky. The server shouldn't shutdown before all
+            var coreLogs = TestSink.Writes.Where(w => w.LoggerName == "Microsoft.AspNetCore.Server.Kestrel");
-            // the connections are closed, yet sometimes the connection stop log isn't observed here.
+            Assert.Single(coreLogs.Where(w => w.EventId == connectionStopEventId));
-            //var coreLogs = TestSink.Writes.Where(w => w.LoggerName == "Microsoft.AspNetCore.Server.Kestrel");
-            //Assert.Single(coreLogs.Where(w => w.EventId == connectionStopEventId));
-
-            Assert.True(requestAborted, "RequestAborted token didn't fire.");
 
             var transportLogs = TestSink.Writes.Where(w => w.LoggerName == "Microsoft.AspNetCore.Server.Kestrel" ||
                                                            w.LoggerName == "Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv" ||
@@ -512,6 +514,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                         appFuncCompleted.SetResult(null);
                         throw;
                     }
+                    finally
+                    {
+                        await requestAborted.Task.DefaultTimeout();
+                    }
                 }
 
                 using (var server = new TestServer(App, testContext, listenOptions))
@@ -607,6 +613,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                     appFuncCompleted.SetResult(null);
                     throw;
                 }
+                finally
+                {
+                    await aborted.Task.DefaultTimeout();
+                }
             }, testContext, listenOptions))
             {
                 using (var connection = server.CreateConnection())
@@ -682,6 +692,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                     copyToAsyncCts.SetException(ex);
                     throw;
                 }
+                finally
+                {
+                    await requestAborted.Task.DefaultTimeout();
+                }
 
                 copyToAsyncCts.SetException(new Exception("This shouldn't be reached."));
             }
@@ -777,12 +791,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                     var targetBytesPerSecond = chunkSize / 4;
                     await AssertStreamCompleted(connection.Stream, minTotalOutputSize, targetBytesPerSecond);
                     await appFuncCompleted.Task.DefaultTimeout();
-
-                    mockKestrelTrace.Verify(t => t.ResponseMinimumDataRateNotSatisfied(It.IsAny<string>(), It.IsAny<string>()), Times.Never());
-                    mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny<string>()), Times.Once());
-                    Assert.False(requestAborted);
                 }
             }
+
+            mockKestrelTrace.Verify(t => t.ResponseMinimumDataRateNotSatisfied(It.IsAny<string>(), It.IsAny<string>()), Times.Never());
+            mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny<string>()), Times.Once());
+            Assert.False(requestAborted);
         }
 
         [Fact]
@@ -852,12 +866,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                     // Make sure consuming a single set of response headers exceeds the 2 second timeout.
                     var targetBytesPerSecond = responseSize / 4;
                     await AssertStreamCompleted(connection.Stream, minTotalOutputSize, targetBytesPerSecond);
-
-                    mockKestrelTrace.Verify(t => t.ResponseMinimumDataRateNotSatisfied(It.IsAny<string>(), It.IsAny<string>()), Times.Never());
-                    mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny<string>()), Times.Once());
-                    Assert.False(requestAborted);
                 }
             }
+
+            mockKestrelTrace.Verify(t => t.ResponseMinimumDataRateNotSatisfied(It.IsAny<string>(), It.IsAny<string>()), Times.Never());
+            mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny<string>()), Times.Once());
+            Assert.False(requestAborted);
         }
 
         private async Task AssertStreamAborted(Stream stream, int totalBytes)

test/shared/TestServiceContext.cs

@@ -77,6 +77,8 @@ namespace Microsoft.AspNetCore.Testing
 
         public Func<MemoryPool<byte>> MemoryPoolFactory { get; set; } = KestrelMemoryPool.Create;
 
+        public int ExpectedConnectionMiddlewareCount { get; set; }
+
         public string DateHeaderValue => DateHeaderValueManager.GetDateHeaderValues().String;
     }
 }

test/shared/TransportTestHelpers/TestServer.cs

@@ -17,6 +17,7 @@ using Microsoft.AspNetCore.Testing;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
+using Xunit;
 
 namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
 {
@@ -48,6 +49,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
             : this(app, context, options => options.ListenOptions.Add(listenOptions), configureServices)
         {
         }
+
         public TestServer(RequestDelegate app, TestServiceContext context, Action<KestrelServerOptions> configureKestrel)
             : this(app, context, configureKestrel, _ => { })
         {
@@ -77,6 +79,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                         {
                             c.Configure(context.ServerOptions);
                         }
+
+                        // Prevent ListenOptions reuse. This is easily done accidentally when trying to debug a test by running it
+                        // in a loop, but will cause problems because only the app func from the first loop will ever be invoked.
+                        Assert.All(context.ServerOptions.ListenOptions, lo =>
+                            Assert.Equal(context.ExpectedConnectionMiddlewareCount, lo._middleware.Count));
+
                         return new KestrelServer(sp.GetRequiredService<ITransportFactory>(), context);
                     });
                     configureServices(services);