diff --git a/src/Servers/Connections.Abstractions/src/ConnectionContext.cs b/src/Servers/Connections.Abstractions/src/ConnectionContext.cs index 680762d680..a709a5f891 100644 --- a/src/Servers/Connections.Abstractions/src/ConnectionContext.cs +++ b/src/Servers/Connections.Abstractions/src/ConnectionContext.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; @@ -26,6 +26,6 @@ namespace Microsoft.AspNetCore.Connections Features.Get()?.Abort(); } - public virtual void Abort() => Abort(new ConnectionAbortedException("The connection was aborted by the application.")); + public virtual void Abort() => Abort(new ConnectionAbortedException("The connection was aborted by the application via ConnectionContext.Abort().")); } } diff --git a/src/Servers/Kestrel/Core/src/CoreStrings.resx b/src/Servers/Kestrel/Core/src/CoreStrings.resx index 774305483a..c90468e5a1 100644 --- a/src/Servers/Kestrel/Core/src/CoreStrings.resx +++ b/src/Servers/Kestrel/Core/src/CoreStrings.resx @@ -590,4 +590,10 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l A frame of type {frameType} was received after stream {streamId} was reset or aborted. - + + HTTP protocol selection failed. + + + Server shutdown started during connection initialization. + + \ No newline at end of file diff --git a/src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs b/src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs index 63a926585a..92d59a66af 100644 --- a/src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs @@ -149,7 +149,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal break; case HttpProtocols.None: // An error was already logged in SelectProtocol(), but we should close the connection. - Abort(ex: null); + Abort(new ConnectionAbortedException(CoreStrings.ProtocolSelectionFailed)); break; default: // SelectProtocol() only returns Http1, Http2 or None. @@ -222,7 +222,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal switch (_protocolSelectionState) { case ProtocolSelectionState.Initializing: - CloseUninitializedConnection(abortReason: null); + CloseUninitializedConnection(new ConnectionAbortedException(CoreStrings.ServerShutdownDuringConnectionInitialization)); _protocolSelectionState = ProtocolSelectionState.Aborted; break; case ProtocolSelectionState.Selected: @@ -241,7 +241,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal switch (_protocolSelectionState) { case ProtocolSelectionState.Initializing: - CloseUninitializedConnection(abortReason: null); + // OnReader/WriterCompleted callbacks are not wired until after leaving the Initializing state. + Debug.Assert(false); + + CloseUninitializedConnection(new ConnectionAbortedException("HttpConnection.OnInputOrOutputCompleted() called while in the ProtocolSelectionState.Initializing state!?")); _protocolSelectionState = ProtocolSelectionState.Aborted; break; case ProtocolSelectionState.Selected: diff --git a/src/Servers/Kestrel/Core/src/Properties/CoreStrings.Designer.cs b/src/Servers/Kestrel/Core/src/Properties/CoreStrings.Designer.cs index 7690976935..592d92e4d3 100644 --- a/src/Servers/Kestrel/Core/src/Properties/CoreStrings.Designer.cs +++ b/src/Servers/Kestrel/Core/src/Properties/CoreStrings.Designer.cs @@ -2212,6 +2212,34 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core internal static string FormatHttp2ErrorStreamAborted(object frameType, object streamId) => string.Format(CultureInfo.CurrentCulture, GetString("Http2ErrorStreamAborted", "frameType", "streamId"), frameType, streamId); + /// + /// HTTP protocol selection failed. + /// + internal static string ProtocolSelectionFailed + { + get => GetString("ProtocolSelectionFailed"); + } + + /// + /// HTTP protocol selection failed. + /// + internal static string FormatProtocolSelectionFailed() + => GetString("ProtocolSelectionFailed"); + + /// + /// Server shutdown started during connection initialization. + /// + internal static string ServerShutdownDuringConnectionInitialization + { + get => GetString("ServerShutdownDuringConnectionInitialization"); + } + + /// + /// Server shutdown started during connection initialization. + /// + internal static string FormatServerShutdownDuringConnectionInitialization() + => GetString("ServerShutdownDuringConnectionInitialization"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Servers/Kestrel/Core/test/ConnectionContextTests.cs b/src/Servers/Kestrel/Core/test/ConnectionContextTests.cs new file mode 100644 index 0000000000..86fdf00a7c --- /dev/null +++ b/src/Servers/Kestrel/Core/test/ConnectionContextTests.cs @@ -0,0 +1,27 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Connections; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests +{ + public class ConnectionContextTests + { + [Fact] + public void ParameterlessAbortCreateConnectionAbortedException() + { + var mockConnectionContext = new Mock { CallBase = true }; + ConnectionAbortedException ex = null; + + mockConnectionContext.Setup(c => c.Abort(It.IsAny())) + .Callback(abortReason => ex = abortReason); + + mockConnectionContext.Object.Abort(); + + Assert.NotNull(ex); + Assert.Equal("The connection was aborted by the application via ConnectionContext.Abort().", ex.Message); + } + } +} diff --git a/src/Servers/Kestrel/Transport.Abstractions/src/Internal/TransportConnection.FeatureCollection.cs b/src/Servers/Kestrel/Transport.Abstractions/src/Internal/TransportConnection.FeatureCollection.cs index 42f151d25d..2a07874ab7 100644 --- a/src/Servers/Kestrel/Transport.Abstractions/src/Internal/TransportConnection.FeatureCollection.cs +++ b/src/Servers/Kestrel/Transport.Abstractions/src/Internal/TransportConnection.FeatureCollection.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.IO.Pipelines; using System.Net; using System.Threading; +using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; @@ -91,7 +92,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal set => ConnectionClosedRequested = value; } - void IConnectionLifetimeFeature.Abort() => Abort(abortReason: null); + void IConnectionLifetimeFeature.Abort() => Abort(new ConnectionAbortedException("The connection was aborted by the application via IConnectionLifetimeFeature.Abort().")); void IConnectionLifetimeNotificationFeature.RequestClose() => RequestClose(); diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ConnectionAdapterTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ConnectionAdapterTests.cs index 3a9a543147..9d426285f8 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ConnectionAdapterTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ConnectionAdapterTests.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal; using Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTransport; using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.Logging.Testing; using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests @@ -124,6 +125,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests } [Fact] + [CollectDump] public async Task ImmediateShutdownAfterOnConnectionAsyncDoesNotCrash() { var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) @@ -135,10 +137,43 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests var stopTask = Task.CompletedTask; using (var server = new TestServer(TestApp.EchoApp, serviceContext, listenOptions)) + using (var shutdownCts = new CancellationTokenSource(TimeSpan.FromSeconds(5))) { using (var connection = server.CreateConnection()) { + // Use the default 5 second shutdown timeout. If it hangs that long, we'll look + // at the collected memory dump. + stopTask = server.StopAsync(shutdownCts.Token); + } + + await stopTask; + } + } + + [Fact] + public async Task ImmediateShutdownDuringOnConnectionAsyncDoesNotCrash() + { + var waitingConnectionAdapter = new WaitingConnectionAdapter(); + var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + { + ConnectionAdapters = { waitingConnectionAdapter } + }; + + var serviceContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(TestApp.EchoApp, serviceContext, listenOptions)) + { + Task stopTask; + + using (var connection = server.CreateConnection()) + { + var closingMessageTask = TestApplicationErrorLogger.WaitForMessage(m => m.Message.Contains(CoreStrings.ServerShutdownDuringConnectionInitialization)); + stopTask = server.StopAsync(); + + await closingMessageTask.DefaultTimeout(); + + waitingConnectionAdapter.Complete(); } await stopTask; @@ -232,6 +267,24 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests } } + private class WaitingConnectionAdapter : IConnectionAdapter + { + private TaskCompletionSource _waitingTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + public bool IsHttps => false; + + public async Task OnConnectionAsync(ConnectionAdapterContext context) + { + await _waitingTcs.Task; + return new AdaptedConnection(context.ConnectionStream); + } + + public void Complete() + { + _waitingTcs.TrySetResult(null); + } + } + private class ThrowingConnectionAdapter : IConnectionAdapter { public bool IsHttps => false; diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index 884f4194ef..24e1287b73 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -4,12 +4,14 @@ using System; using System.Collections.Generic; using System.IO; +using System.IO.Pipelines; using System.Linq; using System.Net; using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core; @@ -2319,6 +2321,62 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests } } + [Fact] + public async Task AppAbortIsLogged() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(httpContext => + { + httpContext.Abort(); + return Task.CompletedTask; + }, testContext)) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.ReceiveEnd(); + } + await server.StopAsync(); + } + + Assert.Single(TestApplicationErrorLogger.Messages.Where(m => m.Message.Contains(CoreStrings.ConnectionAbortedByApplication))); + } + + [Fact] + public async Task AppAbortViaIConnectionLifetimeFeatureIsLogged() + { + // Ensure the response doesn't get flush before the abort is observed by scheduling inline. + var testContext = new TestServiceContext(LoggerFactory) + { + Scheduler = PipeScheduler.Inline + }; + + using (var server = new TestServer(httpContext => + { + httpContext.Features.Get().Abort(); + return Task.CompletedTask; + }, testContext)) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.ReceiveEnd(); + } + await server.StopAsync(); + } + + Assert.Single(TestApplicationErrorLogger.Messages.Where(m => m.Message.Contains("The connection was aborted by the application via IConnectionLifetimeFeature.Abort()."))); + } + [Fact] public async Task ResponseHeadersAreResetOnEachRequest() { diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/TestTransport/TestServer.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/TestTransport/TestServer.cs index 11fe0dd27b..0181f72daf 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/TestTransport/TestServer.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/TestTransport/TestServer.cs @@ -13,6 +13,7 @@ using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Xunit; @@ -68,6 +69,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTrans HttpClientSlim = new InMemoryHttpClientSlim(this); var hostBuilder = new WebHostBuilder() + .UseSetting(WebHostDefaults.ShutdownTimeoutKey, TestConstants.DefaultTimeout.TotalSeconds.ToString()) .ConfigureServices(services => { configureServices(services); @@ -106,9 +108,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTrans return new InMemoryConnection(transportConnection); } - public Task StopAsync() + public Task StopAsync(CancellationToken cancellationToken = default) { - return _host.StopAsync(); + return _host.StopAsync(cancellationToken); } public void Dispose()