diff --git a/benchmarks/Kestrel.Performance/InMemoryTransportBenchmark.cs b/benchmarks/Kestrel.Performance/InMemoryTransportBenchmark.cs index 8440783b71..0e77ddd9c5 100644 --- a/benchmarks/Kestrel.Performance/InMemoryTransportBenchmark.cs +++ b/benchmarks/Kestrel.Performance/InMemoryTransportBenchmark.cs @@ -205,10 +205,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance } } } - - protected override void AbortCore(ConnectionAbortedException abortReason) - { - } } // Copied from https://github.com/aspnet/benchmarks/blob/dev/src/Benchmarks/Middleware/PlaintextMiddleware.cs diff --git a/src/Connections.Abstractions/ConnectionContext.cs b/src/Connections.Abstractions/ConnectionContext.cs index 7c505ba108..bb942f2627 100644 --- a/src/Connections.Abstractions/ConnectionContext.cs +++ b/src/Connections.Abstractions/ConnectionContext.cs @@ -25,5 +25,7 @@ namespace Microsoft.AspNetCore.Connections // ConnectioContext.Abort() Features.Get()?.Abort(); } + + public virtual void Abort() => Abort(new ConnectionAbortedException("The connection was aborted by the application.")); } } diff --git a/src/Connections.Abstractions/DefaultConnectionContext.cs b/src/Connections.Abstractions/DefaultConnectionContext.cs index 4712eef953..56c5d0424f 100644 --- a/src/Connections.Abstractions/DefaultConnectionContext.cs +++ b/src/Connections.Abstractions/DefaultConnectionContext.cs @@ -65,8 +65,6 @@ namespace Microsoft.AspNetCore.Connections public CancellationToken ConnectionClosed { get; set; } - public void Abort() => Abort(abortReason: null); - public override void Abort(ConnectionAbortedException abortReason) { ThreadPool.QueueUserWorkItem(cts => ((CancellationTokenSource)cts).Cancel(), _connectionClosedTokenSource); diff --git a/src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs b/src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs index 2473f529c0..6b15c06705 100644 --- a/src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs +++ b/src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs @@ -87,7 +87,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal (_, state) => ((HttpConnection)state).OnInputOrOutputCompleted(), connection); - await AsTask(lifetimeFeature.ConnectionClosed); + await CancellationTokenAsTask(lifetimeFeature.ConnectionClosed); connection.OnConnectionClosed(); @@ -99,8 +99,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal } } - private Task AsTask(CancellationToken token) + private static Task CancellationTokenAsTask(CancellationToken token) { + if (token.IsCancellationRequested) + { + return Task.CompletedTask; + } + + // Transports already dispatch prior to tripping ConnectionClosed + // since application code can register to this token. var tcs = new TaskCompletionSource(); token.Register(() => tcs.SetResult(null)); return tcs.Task; diff --git a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs index 0631258475..604e497807 100644 --- a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs +++ b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs @@ -55,14 +55,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal public CancellationToken ConnectionClosed { get; set; } - // DO NOT remove this override to ConnectionContext.Abort(). Doing so would cause - // any TransportConnection that does not override Abort() or calls base.Abort() + // DO NOT remove this override to ConnectionContext.Abort. Doing so would cause + // any TransportConnection that does not override Abort or calls base.Abort // to stack overflow when IConnectionLifetimeFeature.Abort() is called. + // That said, all derived types should override this method should override + // this implementation of Abort because canceling pending output reads is not + // sufficient to abort the connection if there is backpressure. public override void Abort(ConnectionAbortedException abortReason) { - AbortCore(abortReason); + Output.CancelPendingRead(); } - - protected abstract void AbortCore(ConnectionAbortedException abortReason); } } diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs index efb696f0f0..b9f0aa0ab4 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs @@ -116,7 +116,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal } } - protected override void AbortCore(ConnectionAbortedException abortReason) + public override void Abort(ConnectionAbortedException abortReason) { _abortReason = abortReason; Output.CancelPendingRead(); diff --git a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs index 5e2d768913..ec9a96e2f7 100644 --- a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs +++ b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs @@ -92,7 +92,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal } } - protected override void AbortCore(ConnectionAbortedException abortReason) + public override void Abort(ConnectionAbortedException abortReason) { _abortReason = abortReason; Output.CancelPendingRead(); diff --git a/test/Kestrel.FunctionalTests/ResponseTests.cs b/test/Kestrel.FunctionalTests/ResponseTests.cs index 558158ba44..b23a5fe868 100644 --- a/test/Kestrel.FunctionalTests/ResponseTests.cs +++ b/test/Kestrel.FunctionalTests/ResponseTests.cs @@ -2459,7 +2459,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests await context.Response.Body.WriteAsync(scratchBuffer, 0, scratchBuffer.Length); await Task.Delay(10); } - + appCompletedTcs.SetResult(null); }, new TestServiceContext(LoggerFactory), listenOptions)) {