From b41c4078bdbbf2b2413fe13922a64552ed1177b6 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Wed, 14 Dec 2016 16:53:19 -0800 Subject: [PATCH] Abort connections not closed during shutdown (#1112). --- .../Internal/Http/Connection.cs | 6 ++-- .../Internal/Http/ConnectionManager.cs | 29 +++++++++++++++---- .../Internal/Http/SocketOutput.cs | 8 ++--- .../Internal/Infrastructure/IKestrelTrace.cs | 2 ++ .../Internal/Infrastructure/KestrelThread.cs | 7 ++++- .../Internal/Infrastructure/KestrelTrace.cs | 7 +++++ test/shared/MockConnection.cs | 4 ++- 7 files changed, 50 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs index 1b7eaa8e97..2b2366ee4e 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Connection.cs @@ -142,7 +142,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http return _socketClosedTcs.Task; } - public virtual void Abort(Exception error = null) + public virtual Task AbortAsync(Exception error = null) { // Frame.Abort calls user code while this method is always // called from a libuv thread. @@ -150,6 +150,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { _frame.Abort(error); }); + + return _socketClosedTcs.Task; } // Called on Libuv thread @@ -285,7 +287,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http if (!normalRead) { - Abort(error); + AbortAsync(error); } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/ConnectionManager.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/ConnectionManager.cs index 561f45ee06..884324f0e8 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/ConnectionManager.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/ConnectionManager.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Internal.Networking; @@ -22,17 +21,37 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } public async Task WalkConnectionsAndCloseAsync(TimeSpan timeout) + { + return await WalkConnectionsAsync((connectionManager, tcs) => connectionManager.WalkConnectionsAndCloseCore(tcs), timeout).ConfigureAwait(false); + } + + public async Task WalkConnectionsAndAbortAsync(TimeSpan timeout) + { + return await WalkConnectionsAsync((connectionManager, tcs) => connectionManager.WalkConnectionsAndAbortCore(tcs), timeout).ConfigureAwait(false); + } + + private async Task WalkConnectionsAsync(Action> action, TimeSpan timeout) { var tcs = new TaskCompletionSource(); - _thread.Post(state => ((ConnectionManager)state).WalkConnectionsAndCloseCore(tcs), this); + _thread.Post(state => action((ConnectionManager)state, tcs), this); return await Task.WhenAny(tcs.Task, Task.Delay(timeout)).ConfigureAwait(false) == tcs.Task; } private void WalkConnectionsAndCloseCore(TaskCompletionSource tcs) { - var connectionStopTasks = new List(); + WalkConnectionsCore(connection => connection.StopAsync(), tcs); + } + + private void WalkConnectionsAndAbortCore(TaskCompletionSource tcs) + { + WalkConnectionsCore(connection => connection.AbortAsync(), tcs); + } + + private void WalkConnectionsCore(Func action, TaskCompletionSource tcs) + { + var tasks = new List(); _thread.Walk(ptr => { @@ -41,13 +60,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http if (connection != null) { - connectionStopTasks.Add(connection.StopAsync()); + tasks.Add(action(connection)); } }); _threadPool.Run(() => { - Task.WaitAll(connectionStopTasks.ToArray()); + Task.WaitAll(tasks.ToArray()); tcs.SetResult(null); }); } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketOutput.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketOutput.cs index 4a6cac5006..d7ac287ec6 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketOutput.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketOutput.cs @@ -158,7 +158,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { if (cancellationToken.IsCancellationRequested) { - _connection.Abort(); + _connection.AbortAsync(); _cancelled = true; return TaskUtilities.GetCancelledTask(cancellationToken); } @@ -304,7 +304,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { // Abort the connection for any failed write // Queued on threadpool so get it in as first op. - _connection.Abort(); + _connection.AbortAsync(); _cancelled = true; CompleteAllWrites(); @@ -374,7 +374,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { // Abort the connection for any failed write // Queued on threadpool so get it in as first op. - _connection.Abort(); + _connection.AbortAsync(); _cancelled = true; _lastWriteError = error; } @@ -532,7 +532,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { if (cancellationToken.IsCancellationRequested) { - _connection.Abort(); + _connection.AbortAsync(); _cancelled = true; return TaskUtilities.GetCancelledTask(cancellationToken); } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/IKestrelTrace.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/IKestrelTrace.cs index 2645aa8e88..ff622fa69b 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/IKestrelTrace.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/IKestrelTrace.cs @@ -43,6 +43,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure void NotAllConnectionsClosedGracefully(); + void NotAllConnectionsAborted(); + void ApplicationError(string connectionId, Exception ex); } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelThread.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelThread.cs index 60536fe5d9..87e48f5205 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelThread.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelThread.cs @@ -165,6 +165,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal if (!await ConnectionManager.WalkConnectionsAndCloseAsync(_shutdownTimeout).ConfigureAwait(false)) { _log.NotAllConnectionsClosedGracefully(); + + if (!await ConnectionManager.WalkConnectionsAndAbortAsync(TimeSpan.FromSeconds(1)).ConfigureAwait(false)) + { + _log.NotAllConnectionsAborted(); + } } var result = await WaitAsync(PostAsync(state => @@ -281,7 +286,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal { lock (_startSync) { - var tcs = (TaskCompletionSource) parameter; + var tcs = (TaskCompletionSource)parameter; try { _loop.Init(_engine.Libuv); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelTrace.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelTrace.cs index 181e6380a2..9dcd15eedc 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelTrace.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelTrace.cs @@ -26,6 +26,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal private static readonly Action _connectionDisconnectedWrite; private static readonly Action _connectionHeadResponseBodyWrite; private static readonly Action _notAllConnectionsClosedGracefully; + private static readonly Action _notAllConnectionsAborted; private static readonly Action _connectionBadRequest; private static readonly Action _connectionReset; private static readonly Action _requestProcessingError; @@ -54,6 +55,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal _connectionHeadResponseBodyWrite = LoggerMessage.Define(LogLevel.Debug, 18, @"Connection id ""{ConnectionId}"" write of ""{count}"" body bytes to non-body HEAD response."); _connectionReset = LoggerMessage.Define(LogLevel.Debug, 19, @"Connection id ""{ConnectionId}"" reset."); _requestProcessingError = LoggerMessage.Define(LogLevel.Information, 20, @"Connection id ""{ConnectionId}"" request processing ended abnormally."); + _notAllConnectionsAborted = LoggerMessage.Define(LogLevel.Debug, 21, "Some connections failed to abort during server shutdown."); } public KestrelTrace(ILogger logger) @@ -149,6 +151,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal _notAllConnectionsClosedGracefully(_logger, null); } + public virtual void NotAllConnectionsAborted() + { + _notAllConnectionsAborted(_logger, null); + } + public void ConnectionBadRequest(string connectionId, BadHttpRequestException ex) { _connectionBadRequest(_logger, connectionId, ex.Message, ex); diff --git a/test/shared/MockConnection.cs b/test/shared/MockConnection.cs index e1025c624c..eff3da34fb 100644 --- a/test/shared/MockConnection.cs +++ b/test/shared/MockConnection.cs @@ -7,6 +7,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel; using Microsoft.AspNetCore.Server.Kestrel.Internal; using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; +using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Testing { @@ -24,9 +25,10 @@ namespace Microsoft.AspNetCore.Testing }; } - public override void Abort(Exception error = null) + public override Task AbortAsync(Exception error = null) { RequestAbortedSource?.Cancel(); + return TaskCache.CompletedTask; } public override void OnSocketClosed()