diff --git a/src/Kestrel.Core/Internal/ConnectionDispatcher.cs b/src/Kestrel.Core/Internal/ConnectionDispatcher.cs index f0c90a4388..780ede5a35 100644 --- a/src/Kestrel.Core/Internal/ConnectionDispatcher.cs +++ b/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); } } diff --git a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs index 07b3be55de..ebcd74a0b2 100644 --- a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs +++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs @@ -39,12 +39,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private Stack, object>> _onStarting; private Stack, 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, - // 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 - // don't have one, and return its token. - var cts = _abortedCts; - return - cts != null ? cts.Token : - (_ioCompleted == 1) ? new CancellationToken(true) : - RequestAbortedSource.Token; + + lock (_abortLock) + { + if (_preventRequestAbortedCancellation) + { + return new CancellationToken(false); + } + + if (_requestAborted) + { + 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; - - // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. - ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this); + if (_abortedCts != null) + { + // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. + 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 name, Span value) { _requestHeadersParsed++; @@ -487,9 +512,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private async Task ProcessRequests(IHttpApplication 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 (_requestRejectedException != null && _ioCompleted == 0) + // If _requestAborted is set, the connection has already been closed. + 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(); diff --git a/test/Kestrel.Core.Tests/Http1ConnectionTests.cs b/test/Kestrel.Core.Tests/Http1ConnectionTests.cs index f2c665828a..39d29e5e59 100644 --- a/test/Kestrel.Core.Tests/Http1ConnectionTests.cs +++ b/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.WaitHandle; + var original = _http1Connection.RequestAborted; foreach (var ch in "hello, worl") { await _http1Connection.WriteAsync(new ArraySegment(new[] { (byte)ch })); - Assert.Same(original, _http1Connection.RequestAborted.WaitHandle); + Assert.Equal(original, _http1Connection.RequestAborted); } await _http1Connection.WriteAsync(new ArraySegment(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.WaitHandle; + var original = _http1Connection.RequestAborted; foreach (var ch in "hello, worl") { await _http1Connection.WriteAsync(new ArraySegment(new[] { (byte)ch }), default(CancellationToken)); - Assert.Same(original, _http1Connection.RequestAborted.WaitHandle); + Assert.Equal(original, _http1Connection.RequestAborted); } await _http1Connection.WriteAsync(new ArraySegment(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.WaitHandle; + var original = _http1Connection.RequestAborted; // Only first write can be WriteAsyncAwaited var startingTask = _http1Connection.InitializeResponseAwaited(Task.CompletedTask, 1); await _http1Connection.WriteAsyncAwaited(startingTask, new ArraySegment(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(new[] { (byte)ch }), default(CancellationToken)); - Assert.Same(original, _http1Connection.RequestAborted.WaitHandle); + Assert.Equal(original, _http1Connection.RequestAborted); } await _http1Connection.WriteAsync(new ArraySegment(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.WaitHandle; + var original = _http1Connection.RequestAborted; _http1Connection.HttpVersion = "HTTP/1.1"; await _http1Connection.WriteAsync(new ArraySegment(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] diff --git a/test/Kestrel.InMemory.FunctionalTests/ConnectionLimitTests.cs b/test/Kestrel.InMemory.FunctionalTests/ConnectionLimitTests.cs index 20c911ac33..39434509a6 100644 --- a/test/Kestrel.InMemory.FunctionalTests/ConnectionLimitTests.cs +++ b/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 => { diff --git a/test/Kestrel.InMemory.FunctionalTests/TestTransport/TestServer.cs b/test/Kestrel.InMemory.FunctionalTests/TestTransport/TestServer.cs index 388b8fa944..17be223833 100644 --- a/test/Kestrel.InMemory.FunctionalTests/TestTransport/TestServer.cs +++ b/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); }); }); diff --git a/test/Kestrel.Transport.FunctionalTests/RequestTests.cs b/test/Kestrel.Transport.FunctionalTests/RequestTests.cs index e3f0b9ebb5..347ea3661f 100644 --- a/test/Kestrel.Transport.FunctionalTests/RequestTests.cs +++ b/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(async () => await readTcs.Task); + await Assert.ThrowsAsync(async () => await readTcs.Task).DefaultTimeout(); // The cancellation token for only the last request should be triggered. var abortedRequestId = await registrationTcs.Task.DefaultTimeout(); diff --git a/test/Kestrel.Transport.FunctionalTests/ResponseTests.cs b/test/Kestrel.Transport.FunctionalTests/ResponseTests.cs index aaa39ba476..a36c0dc89f 100644 --- a/test/Kestrel.Transport.FunctionalTests/ResponseTests.cs +++ b/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(TaskCreationOptions.RunContinuationsAsynchronously); var readCallbackUnwired = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var clientClosedConnection = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var writeTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -291,10 +295,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests using (var server = new TestServer(async context => { - context.RequestAborted.Register(() => - { - requestAborted = true; - }); + context.RequestAborted.Register(() => requestAborted.SetResult(null)); 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()), 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(TaskCreationOptions.RunContinuationsAsynchronously); var appCompletedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var requestAborted = false; var scratchBuffer = new byte[responseBodySegmentSize]; using (var server = new TestServer(async context => { - context.RequestAborted.Register(() => - { - requestAborted = true; - }); + context.RequestAborted.Register(() => requestAborted.SetResult(null)); 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 - // the connections are closed, yet sometimes the connection stop log isn't observed here. - //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 coreLogs = TestSink.Writes.Where(w => w.LoggerName == "Microsoft.AspNetCore.Server.Kestrel"); + Assert.Single(coreLogs.Where(w => w.EventId == connectionStopEventId)); 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(), It.IsAny()), Times.Never()); - mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); - Assert.False(requestAborted); } } + + mockKestrelTrace.Verify(t => t.ResponseMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny()), Times.Never()); + mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), 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(), It.IsAny()), Times.Never()); - mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); - Assert.False(requestAborted); } } + + mockKestrelTrace.Verify(t => t.ResponseMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny()), Times.Never()); + mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); + Assert.False(requestAborted); } private async Task AssertStreamAborted(Stream stream, int totalBytes) diff --git a/test/shared/TestServiceContext.cs b/test/shared/TestServiceContext.cs index 5944eece8b..b0532b874d 100644 --- a/test/shared/TestServiceContext.cs +++ b/test/shared/TestServiceContext.cs @@ -77,6 +77,8 @@ namespace Microsoft.AspNetCore.Testing public Func> MemoryPoolFactory { get; set; } = KestrelMemoryPool.Create; + public int ExpectedConnectionMiddlewareCount { get; set; } + public string DateHeaderValue => DateHeaderValueManager.GetDateHeaderValues().String; } } diff --git a/test/shared/TransportTestHelpers/TestServer.cs b/test/shared/TransportTestHelpers/TestServer.cs index 9d3bb0cf06..276eeaf768 100644 --- a/test/shared/TransportTestHelpers/TestServer.cs +++ b/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 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(), context); }); configureServices(services);