From 68a08635244df0941b5edd4cc07b2fd0a3cbddcd Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 22 Aug 2018 20:35:28 -0700 Subject: [PATCH] General connection management (#2834) This change makes the handling of graceful shutdown work for more than just http scenarios. This should allow us to move TLS further out and should also allow us to start moving things to connection middleware instead of connection adapters. Summary of the things changed/added: - Added IConnectionLifetimeNotificationFeature that represents an attempt to gracefully close the connection that isn't being aborted. This feels pretty awful but we may have to do it. - Moved connection management to the ConnectionDispatcher and out of the HttpConnectionMiddleware - Removed Http from the names of the ConnectionManager and Heartbeat --- .../IConnectionLifetimeNotificationFeature.cs | 14 ++ .../Internal/ConnectionDispatcher.cs | 69 ++++++-- src/Kestrel.Core/Internal/HttpConnection.cs | 153 +++++++++--------- .../Internal/HttpConnectionContext.cs | 1 - .../Internal/HttpConnectionMiddleware.cs | 46 +----- ...nectionManager.cs => ConnectionManager.cs} | 14 +- ...=> ConnectionManagerShutdownExtensions.cs} | 12 +- .../Infrastructure/ConnectionReference.cs | 26 +++ ...eartbeatManager.cs => HeartbeatManager.cs} | 14 +- .../Infrastructure/HttpConnectionReference.cs | 25 --- .../Infrastructure/KestrelConnection.cs | 28 ++++ .../Infrastructure/KestrelEventSource.cs | 11 +- src/Kestrel.Core/Internal/ServiceContext.cs | 2 +- src/Kestrel.Core/KestrelServer.cs | 16 +- .../TransportConnection.FeatureCollection.cs | 15 ++ .../Internal/TransportConnection.Generated.cs | 46 ++++++ .../Internal/TransportConnection.cs | 50 ++++++ .../Internal/ILibuvTrace.cs | 2 + .../Internal/LibuvConnection.cs | 3 +- .../Internal/LibuvTrace.cs | 8 + .../Internal/ISocketsTrace.cs | 2 + .../Internal/SocketConnection.cs | 18 ++- .../Internal/SocketsTrace.cs | 8 + .../ConnectionDispatcherTests.cs | 7 +- .../HttpConnectionManagerTests.cs | 14 +- .../Kestrel.Core.Tests/HttpConnectionTests.cs | 1 - .../HttpsTests.cs | 2 +- .../KeepAliveTimeoutTests.cs | 12 +- .../RequestBodyTimeoutTests.cs | 4 +- .../RequestHeadersTimeoutTests.cs | 8 +- .../RequestTests.cs | 2 +- .../UpgradeTests.cs | 2 +- .../Http2/ShutdownTests.cs | 2 + test/shared/TestApplicationErrorLogger.cs | 6 +- test/shared/TestServiceContext.cs | 4 +- .../TransportConnectionFeatureCollection.cs | 2 + 36 files changed, 423 insertions(+), 226 deletions(-) create mode 100644 src/Connections.Abstractions/Features/IConnectionLifetimeNotificationFeature.cs rename src/Kestrel.Core/Internal/Infrastructure/{HttpConnectionManager.cs => ConnectionManager.cs} (77%) rename src/Kestrel.Core/Internal/Infrastructure/{HttpConnectionManagerShutdownExtensions.cs => ConnectionManagerShutdownExtensions.cs} (80%) create mode 100644 src/Kestrel.Core/Internal/Infrastructure/ConnectionReference.cs rename src/Kestrel.Core/Internal/Infrastructure/{HttpHeartbeatManager.cs => HeartbeatManager.cs} (57%) delete mode 100644 src/Kestrel.Core/Internal/Infrastructure/HttpConnectionReference.cs create mode 100644 src/Kestrel.Core/Internal/Infrastructure/KestrelConnection.cs diff --git a/src/Connections.Abstractions/Features/IConnectionLifetimeNotificationFeature.cs b/src/Connections.Abstractions/Features/IConnectionLifetimeNotificationFeature.cs new file mode 100644 index 0000000000..7828e4dd04 --- /dev/null +++ b/src/Connections.Abstractions/Features/IConnectionLifetimeNotificationFeature.cs @@ -0,0 +1,14 @@ +// 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.Threading; + +namespace Microsoft.AspNetCore.Connections.Features +{ + public interface IConnectionLifetimeNotificationFeature + { + CancellationToken ConnectionClosedRequested { get; set; } + + void RequestClose(); + } +} diff --git a/src/Kestrel.Core/Internal/ConnectionDispatcher.cs b/src/Kestrel.Core/Internal/ConnectionDispatcher.cs index 79614ec51b..20829ebb14 100644 --- a/src/Kestrel.Core/Internal/ConnectionDispatcher.cs +++ b/src/Kestrel.Core/Internal/ConnectionDispatcher.cs @@ -17,6 +17,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { public class ConnectionDispatcher : IConnectionDispatcher { + private static long _lastConnectionId = long.MinValue; + private readonly ServiceContext _serviceContext; private readonly ConnectionDelegate _connectionDelegate; @@ -44,28 +46,51 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal // This *must* be set before returning from OnConnection connection.Application = pair.Application; - return Execute(connection); + return Execute(new KestrelConnection(connection)); } - private async Task Execute(ConnectionContext connectionContext) + private async Task Execute(KestrelConnection connection) { - using (BeginConnectionScope(connectionContext)) + var id = Interlocked.Increment(ref _lastConnectionId); + var connectionContext = connection.TransportConnection; + + try { - try + _serviceContext.ConnectionManager.AddConnection(id, connection); + + Log.ConnectionStart(connectionContext.ConnectionId); + KestrelEventSource.Log.ConnectionStart(connectionContext); + + using (BeginConnectionScope(connectionContext)) { - await _connectionDelegate(connectionContext); - } - catch (Exception ex) - { - Log.LogCritical(0, ex, $"{nameof(ConnectionDispatcher)}.{nameof(Execute)}() {connectionContext.ConnectionId}"); - } - finally - { - // Complete the transport PipeReader and PipeWriter after calling into application code - connectionContext.Transport.Input.Complete(); - connectionContext.Transport.Output.Complete(); + try + { + await _connectionDelegate(connectionContext); + } + catch (Exception ex) + { + Log.LogCritical(0, ex, $"{nameof(ConnectionDispatcher)}.{nameof(Execute)}() {connectionContext.ConnectionId}"); + } + finally + { + // Complete the transport PipeReader and PipeWriter after calling into application code + connectionContext.Transport.Input.Complete(); + connectionContext.Transport.Output.Complete(); + } + + // Wait for the transport to close + await CancellationTokenAsTask(connectionContext.ConnectionClosed); } } + finally + { + connection.Complete(); + + _serviceContext.ConnectionManager.RemoveConnection(id); + + Log.ConnectionStop(connectionContext.ConnectionId); + KestrelEventSource.Log.ConnectionStop(connectionContext); + } } private IDisposable BeginConnectionScope(ConnectionContext connectionContext) @@ -78,6 +103,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal return null; } + 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(state => ((TaskCompletionSource)state).SetResult(null), tcs); + return tcs.Task; + } + // Internal for testing internal static PipeOptions GetInputPipeOptions(ServiceContext serviceContext, MemoryPool memoryPool, PipeScheduler writerScheduler) => new PipeOptions ( diff --git a/src/Kestrel.Core/Internal/HttpConnection.cs b/src/Kestrel.Core/Internal/HttpConnection.cs index 28224a6ac0..d860d36ba7 100644 --- a/src/Kestrel.Core/Internal/HttpConnection.cs +++ b/src/Kestrel.Core/Internal/HttpConnection.cs @@ -11,6 +11,7 @@ using System.Net; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal; @@ -28,8 +29,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal private static readonly ReadOnlyMemory Http2Id = new[] { (byte)'h', (byte)'2' }; private readonly HttpConnectionContext _context; - private readonly TaskCompletionSource _socketClosedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - private readonly TaskCompletionSource _lifetimeTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + private readonly ISystemClock _systemClock; private IList _adaptedConnections; private IDuplexPipe _adaptedTransport; @@ -56,6 +56,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal public HttpConnection(HttpConnectionContext context) { _context = context; + _systemClock = _context.ServiceContext.SystemClock; } // For testing @@ -100,20 +101,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { try { - // TODO: When we start tracking all connection middleware for shutdown, go back - // to logging connections tart and stop in ConnectionDispatcher so we get these - // logs for all connection middleware. - Log.ConnectionStart(ConnectionId); - KestrelEventSource.Log.ConnectionStart(this); - AdaptedPipeline adaptedPipeline = null; var adaptedPipelineTask = Task.CompletedTask; - // _adaptedTransport must be set prior to adding the connection to the manager in order - // to allow the connection to be aported prior to protocol selection. + // _adaptedTransport must be set prior to wiring up callbacks + // to allow the connection to be aborted prior to protocol selection. _adaptedTransport = _context.Transport; - if (_context.ConnectionAdapters.Count > 0) { adaptedPipeline = new AdaptedPipeline(_adaptedTransport, @@ -124,58 +118,79 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _adaptedTransport = adaptedPipeline; } - _lastTimestamp = _context.ServiceContext.SystemClock.UtcNow.Ticks; + // This feature should never be null in Kestrel + var connectionHeartbeatFeature = _context.ConnectionFeatures.Get(); - _context.ConnectionFeatures.Set(this); + Debug.Assert(connectionHeartbeatFeature != null, nameof(IConnectionHeartbeatFeature) + " is missing!"); - if (adaptedPipeline != null) + connectionHeartbeatFeature?.OnHeartbeat(state => ((HttpConnection)state).Tick(), this); + + var connectionLifetimeNotificationFeature = _context.ConnectionFeatures.Get(); + + Debug.Assert(connectionLifetimeNotificationFeature != null, nameof(IConnectionLifetimeNotificationFeature) + " is missing!"); + + using (connectionLifetimeNotificationFeature?.ConnectionClosedRequested.Register(state => ((HttpConnection)state).StopProcessingNextRequest(), this)) { - // Stream can be null here and run async will close the connection in that case - var stream = await ApplyConnectionAdaptersAsync(); - adaptedPipelineTask = adaptedPipeline.RunAsync(stream); - } + _lastTimestamp = _context.ServiceContext.SystemClock.UtcNow.Ticks; - IRequestProcessor requestProcessor = null; + _context.ConnectionFeatures.Set(this); - lock (_protocolSelectionLock) - { - // Ensure that the connection hasn't already been stopped. - if (_protocolSelectionState == ProtocolSelectionState.Initializing) + if (adaptedPipeline != null) { - switch (SelectProtocol()) - { - case HttpProtocols.Http1: - // _http1Connection must be initialized before adding the connection to the connection manager - requestProcessor = _http1Connection = CreateHttp1Connection(_adaptedTransport); - _protocolSelectionState = ProtocolSelectionState.Selected; - break; - case HttpProtocols.Http2: - // _http2Connection must be initialized before yielding control to the transport thread, - // to prevent a race condition where _http2Connection.Abort() is called just as - // _http2Connection is about to be initialized. - requestProcessor = CreateHttp2Connection(_adaptedTransport); - _protocolSelectionState = ProtocolSelectionState.Selected; - break; - case HttpProtocols.None: - // An error was already logged in SelectProtocol(), but we should close the connection. - Abort(ex: null); - break; - default: - // SelectProtocol() only returns Http1, Http2 or None. - throw new NotSupportedException($"{nameof(SelectProtocol)} returned something other than Http1, Http2 or None."); - } - - _requestProcessor = requestProcessor; + // Stream can be null here and run async will close the connection in that case + var stream = await ApplyConnectionAdaptersAsync(); + adaptedPipelineTask = adaptedPipeline.RunAsync(stream); } - } - if (requestProcessor != null) - { - await requestProcessor.ProcessRequestsAsync(httpApplication); - } + IRequestProcessor requestProcessor = null; - await adaptedPipelineTask; - await _socketClosedTcs.Task; + lock (_protocolSelectionLock) + { + // Ensure that the connection hasn't already been stopped. + if (_protocolSelectionState == ProtocolSelectionState.Initializing) + { + switch (SelectProtocol()) + { + case HttpProtocols.Http1: + // _http1Connection must be initialized before adding the connection to the connection manager + requestProcessor = _http1Connection = CreateHttp1Connection(_adaptedTransport); + _protocolSelectionState = ProtocolSelectionState.Selected; + break; + case HttpProtocols.Http2: + // _http2Connection must be initialized before yielding control to the transport thread, + // to prevent a race condition where _http2Connection.Abort() is called just as + // _http2Connection is about to be initialized. + requestProcessor = CreateHttp2Connection(_adaptedTransport); + _protocolSelectionState = ProtocolSelectionState.Selected; + break; + case HttpProtocols.None: + // An error was already logged in SelectProtocol(), but we should close the connection. + Abort(ex: null); + break; + default: + // SelectProtocol() only returns Http1, Http2 or None. + throw new NotSupportedException($"{nameof(SelectProtocol)} returned something other than Http1, Http2 or None."); + } + + _requestProcessor = requestProcessor; + } + } + + _context.Transport.Input.OnWriterCompleted( + (_, state) => ((HttpConnection)state).OnInputOrOutputCompleted(), + this); + + _context.Transport.Output.OnReaderCompleted( + (_, state) => ((HttpConnection)state).OnInputOrOutputCompleted(), + this); + + if (requestProcessor != null) + { + await requestProcessor.ProcessRequestsAsync(httpApplication); + } + + await adaptedPipelineTask; + } } catch (Exception ex) { @@ -189,11 +204,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { _context.ServiceContext.ConnectionManager.UpgradedConnectionCount.ReleaseOne(); } - - Log.ConnectionStop(ConnectionId); - KestrelEventSource.Log.ConnectionStop(this); - - _lifetimeTcs.SetResult(null); } } @@ -235,12 +245,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal }); } - public void OnConnectionClosed() - { - _socketClosedTcs.TrySetResult(null); - } - - public Task StopProcessingNextRequestAsync() + private void StopProcessingNextRequest() { lock (_protocolSelectionLock) { @@ -257,11 +262,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal break; } } - - return _lifetimeTcs.Task; } - public void OnInputOrOutputCompleted() + private void OnInputOrOutputCompleted() { lock (_protocolSelectionLock) { @@ -281,7 +284,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal } } - public void Abort(ConnectionAbortedException ex) + private void Abort(ConnectionAbortedException ex) { lock (_protocolSelectionLock) { @@ -301,13 +304,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal } } - public Task AbortAsync(ConnectionAbortedException ex) - { - Abort(ex); - - return _socketClosedTcs.Task; - } - private async Task ApplyConnectionAdaptersAsync() { var connectionAdapters = _context.ConnectionAdapters; @@ -381,6 +377,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal return http2Enabled && (!hasTls || Http2Id.Span.SequenceEqual(applicationProtocol.Span)) ? HttpProtocols.Http2 : HttpProtocols.Http1; } + private void Tick() + { + Tick(_systemClock.UtcNow); + } + public void Tick(DateTimeOffset now) { if (_protocolSelectionState == ProtocolSelectionState.Aborted) diff --git a/src/Kestrel.Core/Internal/HttpConnectionContext.cs b/src/Kestrel.Core/Internal/HttpConnectionContext.cs index da01a5a7b0..c5eceead0a 100644 --- a/src/Kestrel.Core/Internal/HttpConnectionContext.cs +++ b/src/Kestrel.Core/Internal/HttpConnectionContext.cs @@ -14,7 +14,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal public class HttpConnectionContext { public string ConnectionId { get; set; } - public long HttpConnectionId { get; set; } public HttpProtocols Protocols { get; set; } public ConnectionContext ConnectionContext { get; set; } public ServiceContext ServiceContext { get; set; } diff --git a/src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs b/src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs index 5d05220811..83bca08997 100644 --- a/src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs +++ b/src/Kestrel.Core/Internal/HttpConnectionMiddleware.cs @@ -16,8 +16,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal { public class HttpConnectionMiddleware { - private static long _lastHttpConnectionId = long.MinValue; - private readonly IList _connectionAdapters; private readonly ServiceContext _serviceContext; private readonly IHttpApplication _application; @@ -33,19 +31,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _connectionAdapters = adapters; } - public async Task OnConnectionAsync(ConnectionContext connectionContext) + public Task OnConnectionAsync(ConnectionContext connectionContext) { // We need the transport feature so that we can cancel the output reader that the transport is using // This is a bit of a hack but it preserves the existing semantics var memoryPoolFeature = connectionContext.Features.Get(); - var httpConnectionId = Interlocked.Increment(ref _lastHttpConnectionId); - var httpConnectionContext = new HttpConnectionContext { ConnectionId = connectionContext.ConnectionId, ConnectionContext = connectionContext, - HttpConnectionId = httpConnectionId, Protocols = _protocols, ServiceContext = _serviceContext, ConnectionFeatures = connectionContext.Features, @@ -55,7 +50,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal }; var connectionFeature = connectionContext.Features.Get(); - var lifetimeFeature = connectionContext.Features.Get(); if (connectionFeature != null) { @@ -71,44 +65,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal } var connection = new HttpConnection(httpConnectionContext); - _serviceContext.ConnectionManager.AddConnection(httpConnectionId, connection); - try - { - var processingTask = connection.ProcessRequestsAsync(_application); - - connectionContext.Transport.Input.OnWriterCompleted( - (_, state) => ((HttpConnection)state).OnInputOrOutputCompleted(), - connection); - - connectionContext.Transport.Output.OnReaderCompleted( - (_, state) => ((HttpConnection)state).OnInputOrOutputCompleted(), - connection); - - await CancellationTokenAsTask(lifetimeFeature.ConnectionClosed); - - connection.OnConnectionClosed(); - - await processingTask; - } - finally - { - _serviceContext.ConnectionManager.RemoveConnection(httpConnectionId); - } - } - - 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; + return connection.ProcessRequestsAsync(_application); } } } diff --git a/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionManager.cs b/src/Kestrel.Core/Internal/Infrastructure/ConnectionManager.cs similarity index 77% rename from src/Kestrel.Core/Internal/Infrastructure/HttpConnectionManager.cs rename to src/Kestrel.Core/Internal/Infrastructure/ConnectionManager.cs index feb3a4cb29..bd94ce5658 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionManager.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/ConnectionManager.cs @@ -6,17 +6,17 @@ using System.Collections.Concurrent; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { - public class HttpConnectionManager + public class ConnectionManager { - private readonly ConcurrentDictionary _connectionReferences = new ConcurrentDictionary(); + private readonly ConcurrentDictionary _connectionReferences = new ConcurrentDictionary(); private readonly IKestrelTrace _trace; - public HttpConnectionManager(IKestrelTrace trace, long? upgradedConnectionLimit) + public ConnectionManager(IKestrelTrace trace, long? upgradedConnectionLimit) : this(trace, GetCounter(upgradedConnectionLimit)) { } - public HttpConnectionManager(IKestrelTrace trace, ResourceCounter upgradedConnections) + public ConnectionManager(IKestrelTrace trace, ResourceCounter upgradedConnections) { UpgradedConnectionCount = upgradedConnections; _trace = trace; @@ -27,9 +27,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure /// public ResourceCounter UpgradedConnectionCount { get; } - public void AddConnection(long id, HttpConnection connection) + public void AddConnection(long id, KestrelConnection connection) { - if (!_connectionReferences.TryAdd(id, new HttpConnectionReference(connection))) + if (!_connectionReferences.TryAdd(id, new ConnectionReference(connection))) { throw new ArgumentException(nameof(id)); } @@ -43,7 +43,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure } } - public void Walk(Action callback) + public void Walk(Action callback) { foreach (var kvp in _connectionReferences) { diff --git a/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionManagerShutdownExtensions.cs b/src/Kestrel.Core/Internal/Infrastructure/ConnectionManagerShutdownExtensions.cs similarity index 80% rename from src/Kestrel.Core/Internal/Infrastructure/HttpConnectionManagerShutdownExtensions.cs rename to src/Kestrel.Core/Internal/Infrastructure/ConnectionManagerShutdownExtensions.cs index 1b601c919b..928db83c92 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionManagerShutdownExtensions.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/ConnectionManagerShutdownExtensions.cs @@ -8,29 +8,31 @@ using Microsoft.AspNetCore.Connections; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { - public static class HttpConnectionManagerShutdownExtensions + public static class ConnectionManagerShutdownExtensions { - public static async Task CloseAllConnectionsAsync(this HttpConnectionManager connectionManager, CancellationToken token) + public static async Task CloseAllConnectionsAsync(this ConnectionManager connectionManager, CancellationToken token) { var closeTasks = new List(); connectionManager.Walk(connection => { - closeTasks.Add(connection.StopProcessingNextRequestAsync()); + connection.TransportConnection.RequestClose(); + closeTasks.Add(connection.ExecutionTask); }); var allClosedTask = Task.WhenAll(closeTasks.ToArray()); return await Task.WhenAny(allClosedTask, CancellationTokenAsTask(token)).ConfigureAwait(false) == allClosedTask; } - public static async Task AbortAllConnectionsAsync(this HttpConnectionManager connectionManager) + public static async Task AbortAllConnectionsAsync(this ConnectionManager connectionManager) { var abortTasks = new List(); var canceledException = new ConnectionAbortedException(CoreStrings.ConnectionAbortedDuringServerShutdown); connectionManager.Walk(connection => { - abortTasks.Add(connection.AbortAsync(canceledException)); + connection.TransportConnection.Abort(canceledException); + abortTasks.Add(connection.ExecutionTask); }); var allAbortedTask = Task.WhenAll(abortTasks.ToArray()); diff --git a/src/Kestrel.Core/Internal/Infrastructure/ConnectionReference.cs b/src/Kestrel.Core/Internal/Infrastructure/ConnectionReference.cs new file mode 100644 index 0000000000..8525fd724e --- /dev/null +++ b/src/Kestrel.Core/Internal/Infrastructure/ConnectionReference.cs @@ -0,0 +1,26 @@ +// 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; +using Microsoft.AspNetCore.Connections; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure +{ + public class ConnectionReference + { + private readonly WeakReference _weakReference; + + public ConnectionReference(KestrelConnection connection) + { + _weakReference = new WeakReference(connection); + ConnectionId = connection.TransportConnection.ConnectionId; + } + + public string ConnectionId { get; } + + public bool TryGetConnection(out KestrelConnection connection) + { + return _weakReference.TryGetTarget(out connection); + } + } +} diff --git a/src/Kestrel.Core/Internal/Infrastructure/HttpHeartbeatManager.cs b/src/Kestrel.Core/Internal/Infrastructure/HeartbeatManager.cs similarity index 57% rename from src/Kestrel.Core/Internal/Infrastructure/HttpHeartbeatManager.cs rename to src/Kestrel.Core/Internal/Infrastructure/HeartbeatManager.cs index fba8e8e1a1..bc54e73385 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/HttpHeartbeatManager.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/HeartbeatManager.cs @@ -5,27 +5,29 @@ using System; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { - public class HttpHeartbeatManager : IHeartbeatHandler + public class HeartbeatManager : IHeartbeatHandler, ISystemClock { - private readonly HttpConnectionManager _connectionManager; - private readonly Action _walkCallback; + private readonly ConnectionManager _connectionManager; + private readonly Action _walkCallback; private DateTimeOffset _now; - public HttpHeartbeatManager(HttpConnectionManager connectionManager) + public HeartbeatManager(ConnectionManager connectionManager) { _connectionManager = connectionManager; _walkCallback = WalkCallback; } + public DateTimeOffset UtcNow => _now; + public void OnHeartbeat(DateTimeOffset now) { _now = now; _connectionManager.Walk(_walkCallback); } - private void WalkCallback(HttpConnection connection) + private void WalkCallback(KestrelConnection connection) { - connection.Tick(_now); + connection.TransportConnection.TickHeartbeat(); } } } diff --git a/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionReference.cs b/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionReference.cs deleted file mode 100644 index 395b58a9c4..0000000000 --- a/src/Kestrel.Core/Internal/Infrastructure/HttpConnectionReference.cs +++ /dev/null @@ -1,25 +0,0 @@ -// 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; - -namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure -{ - public class HttpConnectionReference - { - private readonly WeakReference _weakReference; - - public HttpConnectionReference(HttpConnection connection) - { - _weakReference = new WeakReference(connection); - ConnectionId = connection.ConnectionId; - } - - public string ConnectionId { get; } - - public bool TryGetConnection(out HttpConnection connection) - { - return _weakReference.TryGetTarget(out connection); - } - } -} diff --git a/src/Kestrel.Core/Internal/Infrastructure/KestrelConnection.cs b/src/Kestrel.Core/Internal/Infrastructure/KestrelConnection.cs new file mode 100644 index 0000000000..f6c9d537b7 --- /dev/null +++ b/src/Kestrel.Core/Internal/Infrastructure/KestrelConnection.cs @@ -0,0 +1,28 @@ +// 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.Diagnostics.Tracing; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure +{ + public class KestrelConnection + { + private TaskCompletionSource _executionTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + public KestrelConnection(TransportConnection transportConnection) + { + TransportConnection = transportConnection; + ExecutionTask = _executionTcs.Task; + } + + public TransportConnection TransportConnection { get; } + + public Task ExecutionTask { get; } + + internal void Complete() => _executionTcs.TrySetResult(null); + } +} diff --git a/src/Kestrel.Core/Internal/Infrastructure/KestrelEventSource.cs b/src/Kestrel.Core/Internal/Infrastructure/KestrelEventSource.cs index 7faf486d1f..c14138f0c8 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/KestrelEventSource.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/KestrelEventSource.cs @@ -2,8 +2,11 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Diagnostics.Tracing; +using System.Net; using System.Runtime.CompilerServices; +using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; +using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { @@ -25,15 +28,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure // - Avoid renaming methods or parameters marked with EventAttribute. EventSource uses these to form the event object. [NonEvent] - public void ConnectionStart(HttpConnection connection) + public void ConnectionStart(TransportConnection connection) { // avoid allocating strings unless this event source is enabled if (IsEnabled()) { ConnectionStart( connection.ConnectionId, - connection.LocalEndPoint?.ToString(), - connection.RemoteEndPoint?.ToString()); + connection.LocalAddress != null ? new IPEndPoint(connection.LocalAddress, connection.LocalPort).ToString() : null, + connection.RemoteAddress != null ? new IPEndPoint(connection.RemoteAddress, connection.RemotePort).ToString() : null); } } @@ -52,7 +55,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure } [NonEvent] - public void ConnectionStop(HttpConnection connection) + public void ConnectionStop(TransportConnection connection) { if (IsEnabled()) { diff --git a/src/Kestrel.Core/Internal/ServiceContext.cs b/src/Kestrel.Core/Internal/ServiceContext.cs index 1ca1beb237..270640db2a 100644 --- a/src/Kestrel.Core/Internal/ServiceContext.cs +++ b/src/Kestrel.Core/Internal/ServiceContext.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal public DateHeaderValueManager DateHeaderValueManager { get; set; } - public HttpConnectionManager ConnectionManager { get; set; } + public ConnectionManager ConnectionManager { get; set; } public Heartbeat Heartbeat { get; set; } diff --git a/src/Kestrel.Core/KestrelServer.cs b/src/Kestrel.Core/KestrelServer.cs index a6cea3cc0f..cc21f738d0 100644 --- a/src/Kestrel.Core/KestrelServer.cs +++ b/src/Kestrel.Core/KestrelServer.cs @@ -67,17 +67,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core var serverOptions = options.Value ?? new KestrelServerOptions(); var logger = loggerFactory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel"); var trace = new KestrelTrace(logger); - var connectionManager = new HttpConnectionManager( + var connectionManager = new ConnectionManager( trace, serverOptions.Limits.MaxConcurrentUpgradedConnections); - var systemClock = new SystemClock(); - var dateHeaderValueManager = new DateHeaderValueManager(systemClock); - - var httpHeartbeatManager = new HttpHeartbeatManager(connectionManager); + var heartbeatManager = new HeartbeatManager(connectionManager); + var dateHeaderValueManager = new DateHeaderValueManager(heartbeatManager); var heartbeat = new Heartbeat( - new IHeartbeatHandler[] { dateHeaderValueManager, httpHeartbeatManager }, - systemClock, + new IHeartbeatHandler[] { dateHeaderValueManager, heartbeatManager }, + new SystemClock(), DebuggerWrapper.Singleton, trace); @@ -102,7 +100,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core Log = trace, HttpParser = new HttpParser(trace.IsEnabled(LogLevel.Information)), Scheduler = scheduler, - SystemClock = systemClock, + SystemClock = heartbeatManager, DateHeaderValueManager = dateHeaderValueManager, ConnectionManager = connectionManager, Heartbeat = heartbeat, @@ -118,7 +116,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core private IKestrelTrace Trace => ServiceContext.Log; - private HttpConnectionManager ConnectionManager => ServiceContext.ConnectionManager; + private ConnectionManager ConnectionManager => ServiceContext.ConnectionManager; public async Task StartAsync(IHttpApplication application, CancellationToken cancellationToken) { diff --git a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.FeatureCollection.cs b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.FeatureCollection.cs index 63f20ad51f..f88b7189f7 100644 --- a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.FeatureCollection.cs +++ b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.FeatureCollection.cs @@ -20,6 +20,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal IApplicationTransportFeature, ITransportSchedulerFeature, IConnectionLifetimeFeature, + IConnectionHeartbeatFeature, + IConnectionLifetimeNotificationFeature, IBytesWrittenFeature { // NOTE: When feature interfaces are added to or removed from this TransportConnection class implementation, @@ -85,8 +87,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal set => ConnectionClosed = value; } + CancellationToken IConnectionLifetimeNotificationFeature.ConnectionClosedRequested + { + get => ConnectionClosedRequested; + set => ConnectionClosedRequested = value; + } + void IConnectionLifetimeFeature.Abort() => Abort(abortReason: null); + void IConnectionLifetimeNotificationFeature.RequestClose() => RequestClose(); + + void IConnectionHeartbeatFeature.OnHeartbeat(System.Action action, object state) + { + OnHeartbeat(action, state); + } + long IBytesWrittenFeature.TotalBytesWritten => TotalBytesWritten; } } diff --git a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.Generated.cs b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.Generated.cs index 3a91523b9f..2dffb40067 100644 --- a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.Generated.cs +++ b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.Generated.cs @@ -20,6 +20,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal private static readonly Type IApplicationTransportFeatureType = typeof(IApplicationTransportFeature); private static readonly Type ITransportSchedulerFeatureType = typeof(ITransportSchedulerFeature); private static readonly Type IConnectionLifetimeFeatureType = typeof(IConnectionLifetimeFeature); + private static readonly Type IConnectionHeartbeatFeatureType = typeof(IConnectionHeartbeatFeature); + private static readonly Type IConnectionLifetimeNotificationFeatureType = typeof(IConnectionLifetimeNotificationFeature); private static readonly Type IBytesWrittenFeatureType = typeof(IBytesWrittenFeature); private object _currentIHttpConnectionFeature; @@ -30,6 +32,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal private object _currentIApplicationTransportFeature; private object _currentITransportSchedulerFeature; private object _currentIConnectionLifetimeFeature; + private object _currentIConnectionHeartbeatFeature; + private object _currentIConnectionLifetimeNotificationFeature; private object _currentIBytesWrittenFeature; private int _featureRevision; @@ -46,6 +50,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal _currentIApplicationTransportFeature = this; _currentITransportSchedulerFeature = this; _currentIConnectionLifetimeFeature = this; + _currentIConnectionHeartbeatFeature = this; + _currentIConnectionLifetimeNotificationFeature = this; _currentIBytesWrittenFeature = this; } @@ -134,6 +140,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { feature = _currentIConnectionLifetimeFeature; } + else if (key == IConnectionHeartbeatFeatureType) + { + feature = _currentIConnectionHeartbeatFeature; + } + else if (key == IConnectionLifetimeNotificationFeatureType) + { + feature = _currentIConnectionLifetimeNotificationFeature; + } else if (key == IBytesWrittenFeatureType) { feature = _currentIBytesWrittenFeature; @@ -182,6 +196,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { _currentIConnectionLifetimeFeature = value; } + else if (key == IConnectionHeartbeatFeatureType) + { + _currentIConnectionHeartbeatFeature = value; + } + else if (key == IConnectionLifetimeNotificationFeatureType) + { + _currentIConnectionLifetimeNotificationFeature = value; + } else if (key == IBytesWrittenFeatureType) { _currentIBytesWrittenFeature = value; @@ -228,6 +250,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { feature = (TFeature)_currentIConnectionLifetimeFeature; } + else if (typeof(TFeature) == typeof(IConnectionHeartbeatFeature)) + { + feature = (TFeature)_currentIConnectionHeartbeatFeature; + } + else if (typeof(TFeature) == typeof(IConnectionLifetimeNotificationFeature)) + { + feature = (TFeature)_currentIConnectionLifetimeNotificationFeature; + } else if (typeof(TFeature) == typeof(IBytesWrittenFeature)) { feature = (TFeature)_currentIBytesWrittenFeature; @@ -275,6 +305,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { _currentIConnectionLifetimeFeature = feature; } + else if (typeof(TFeature) == typeof(IConnectionHeartbeatFeature)) + { + _currentIConnectionHeartbeatFeature = feature; + } + else if (typeof(TFeature) == typeof(IConnectionLifetimeNotificationFeature)) + { + _currentIConnectionLifetimeNotificationFeature = feature; + } else if (typeof(TFeature) == typeof(IBytesWrittenFeature)) { _currentIBytesWrittenFeature = feature; @@ -319,6 +357,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { yield return new KeyValuePair(IConnectionLifetimeFeatureType, _currentIConnectionLifetimeFeature); } + if (_currentIConnectionHeartbeatFeature != null) + { + yield return new KeyValuePair(IConnectionHeartbeatFeatureType, _currentIConnectionHeartbeatFeature); + } + if (_currentIConnectionLifetimeNotificationFeature != null) + { + yield return new KeyValuePair(IConnectionLifetimeNotificationFeatureType, _currentIConnectionLifetimeNotificationFeature); + } if (_currentIBytesWrittenFeature != null) { yield return new KeyValuePair(IBytesWrittenFeatureType, _currentIBytesWrittenFeature); diff --git a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs index 604e497807..a620a6e5d3 100644 --- a/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs +++ b/src/Kestrel.Transport.Abstractions/Internal/TransportConnection.cs @@ -1,6 +1,7 @@ // 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; using System.Buffers; using System.Collections.Generic; using System.IO.Pipelines; @@ -14,10 +15,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal public abstract partial class TransportConnection : ConnectionContext { private IDictionary _items; + private List<(Action handler, object state)> _heartbeatHandlers; + private readonly object _heartbeatLock = new object(); + protected readonly CancellationTokenSource _connectionClosingCts = new CancellationTokenSource(); public TransportConnection() { FastReset(); + + ConnectionClosedRequested = _connectionClosingCts.Token; } public IPAddress RemoteAddress { get; set; } @@ -55,6 +61,37 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal public CancellationToken ConnectionClosed { get; set; } + public CancellationToken ConnectionClosedRequested { get; set; } + + public void TickHeartbeat() + { + lock (_heartbeatLock) + { + if (_heartbeatHandlers == null) + { + return; + } + + foreach (var (handler, state) in _heartbeatHandlers) + { + handler(state); + } + } + } + + public void OnHeartbeat(Action action, object state) + { + lock (_heartbeatLock) + { + if (_heartbeatHandlers == null) + { + _heartbeatHandlers = new List<(Action handler, object state)>(); + } + + _heartbeatHandlers.Add((action, state)); + } + } + // 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. @@ -65,5 +102,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal { Output.CancelPendingRead(); } + + public void RequestClose() + { + try + { + _connectionClosingCts.Cancel(); + } + catch (ObjectDisposedException) + { + // There's a race where the token could be disposed + // swallow the exception and no-op + } + } } } diff --git a/src/Kestrel.Transport.Libuv/Internal/ILibuvTrace.cs b/src/Kestrel.Transport.Libuv/Internal/ILibuvTrace.cs index 46512ba482..db282bf4b2 100644 --- a/src/Kestrel.Transport.Libuv/Internal/ILibuvTrace.cs +++ b/src/Kestrel.Transport.Libuv/Internal/ILibuvTrace.cs @@ -27,5 +27,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal void ConnectionPause(string connectionId); void ConnectionResume(string connectionId); + + void ConnectionAborted(string connectionId); } } diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs index b9f0aa0ab4..24eeaf6d88 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs @@ -120,7 +120,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal { _abortReason = abortReason; Output.CancelPendingRead(); - + // This cancels any pending I/O. Thread.Post(s => s.Dispose(), _socket); } @@ -129,6 +129,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal public void Dispose() { _connectionClosedTokenSource.Dispose(); + _connectionClosingCts.Dispose(); } // Called on Libuv thread diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs index 96f3338821..14e42c6002 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvTrace.cs @@ -35,6 +35,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal private static readonly Action _connectionReset = LoggerMessage.Define(LogLevel.Debug, 19, @"Connection id ""{ConnectionId}"" reset."); + private static readonly Action _connectionAborted = + LoggerMessage.Define(LogLevel.Debug, new EventId(20, nameof(ConnectionAborted)), @"Connection id ""{ConnectionId}"" aborted."); + private readonly ILogger _logger; public LibuvTrace(ILogger logger) @@ -95,6 +98,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal _connectionResume(_logger, connectionId, null); } + public void ConnectionAborted(string connectionId) + { + _connectionAborted(_logger, connectionId, null); + } + public IDisposable BeginScope(TState state) => _logger.BeginScope(state); public bool IsEnabled(LogLevel logLevel) => _logger.IsEnabled(logLevel); diff --git a/src/Kestrel.Transport.Sockets/Internal/ISocketsTrace.cs b/src/Kestrel.Transport.Sockets/Internal/ISocketsTrace.cs index 06c2c36f20..d08408e2b0 100644 --- a/src/Kestrel.Transport.Sockets/Internal/ISocketsTrace.cs +++ b/src/Kestrel.Transport.Sockets/Internal/ISocketsTrace.cs @@ -19,5 +19,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal void ConnectionPause(string connectionId); void ConnectionResume(string connectionId); + + void ConnectionAborted(string connectionId); } } diff --git a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs index 4cba0b3611..20c301a97f 100644 --- a/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs +++ b/src/Kestrel.Transport.Sockets/Internal/SocketConnection.cs @@ -95,6 +95,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal public override void Abort(ConnectionAbortedException abortReason) { + _trace.ConnectionAborted(ConnectionId); + _abortReason = abortReason; Output.CancelPendingRead(); @@ -106,6 +108,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal public void Dispose() { _connectionClosedTokenSource.Dispose(); + _connectionClosingCts.Dispose(); } private async Task DoReceive() @@ -188,17 +191,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal var flushTask = Input.FlushAsync(); - if (!flushTask.IsCompleted) + var paused = !flushTask.IsCompleted; + + if (paused) { _trace.ConnectionPause(ConnectionId); + } - await flushTask; + var result = await flushTask; + if (paused) + { _trace.ConnectionResume(ConnectionId); } - var result = flushTask.GetAwaiter().GetResult(); - if (result.IsCompleted) + if (result.IsCompleted || result.IsCanceled) { // Pipe consumer is shut down, do we stop writing break; @@ -244,6 +251,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal // Complete the output after disposing the socket Output.Complete(error); + + // Cancel any pending flushes so that the input loop is un-paused + Input.CancelPendingFlush(); } } diff --git a/src/Kestrel.Transport.Sockets/Internal/SocketsTrace.cs b/src/Kestrel.Transport.Sockets/Internal/SocketsTrace.cs index e8aa5fa286..5b132ccf9c 100644 --- a/src/Kestrel.Transport.Sockets/Internal/SocketsTrace.cs +++ b/src/Kestrel.Transport.Sockets/Internal/SocketsTrace.cs @@ -28,6 +28,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal private static readonly Action _connectionReset = LoggerMessage.Define(LogLevel.Debug, new EventId(19, nameof(ConnectionReset)), @"Connection id ""{ConnectionId}"" reset."); + private static readonly Action _connectionAborted = + LoggerMessage.Define(LogLevel.Debug, new EventId(20, nameof(ConnectionAborted)), @"Connection id ""{ConnectionId}"" aborted."); + private readonly ILogger _logger; public SocketsTrace(ILogger logger) @@ -83,6 +86,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal _connectionResume(_logger, connectionId, null); } + public void ConnectionAborted(string connectionId) + { + _connectionAborted(_logger, connectionId, null); + } + public IDisposable BeginScope(TState state) => _logger.BeginScope(state); public bool IsEnabled(LogLevel logLevel) => _logger.IsEnabled(logLevel); diff --git a/test/Kestrel.Core.Tests/ConnectionDispatcherTests.cs b/test/Kestrel.Core.Tests/ConnectionDispatcherTests.cs index dbf838f562..a441423b7d 100644 --- a/test/Kestrel.Core.Tests/ConnectionDispatcherTests.cs +++ b/test/Kestrel.Core.Tests/ConnectionDispatcherTests.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.IO.Pipelines; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; @@ -23,7 +24,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var tcs = new TaskCompletionSource(); var dispatcher = new ConnectionDispatcher(serviceContext, _ => tcs.Task); - var connection = Mock.Of(); + var connection = new Mock() { CallBase = true }.Object; + connection.ConnectionClosed = new CancellationToken(canceled: true); dispatcher.OnConnection(connection); @@ -51,7 +53,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var serviceContext = new TestServiceContext(); var dispatcher = new ConnectionDispatcher(serviceContext, _ => Task.CompletedTask); - var mockConnection = new Mock(); + var mockConnection = new Mock() { CallBase = true }; + mockConnection.Object.ConnectionClosed = new CancellationToken(canceled: true); var mockPipeReader = new Mock(); var mockPipeWriter = new Mock(); var mockPipe = new Mock(); diff --git a/test/Kestrel.Core.Tests/HttpConnectionManagerTests.cs b/test/Kestrel.Core.Tests/HttpConnectionManagerTests.cs index 015aa75881..cb201672d0 100644 --- a/test/Kestrel.Core.Tests/HttpConnectionManagerTests.cs +++ b/test/Kestrel.Core.Tests/HttpConnectionManagerTests.cs @@ -3,8 +3,10 @@ using System; using System.Runtime.CompilerServices; +using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.AspNetCore.Testing; using Moq; using Xunit; @@ -18,7 +20,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { var connectionId = "0"; var trace = new Mock(); - var httpConnectionManager = new HttpConnectionManager(trace.Object, ResourceCounter.Unlimited); + var httpConnectionManager = new ConnectionManager(trace.Object, ResourceCounter.Unlimited); // Create HttpConnection in inner scope so it doesn't get rooted by the current frame. UnrootedConnectionsGetRemovedFromHeartbeatInnerScope(connectionId, httpConnectionManager, trace); @@ -36,14 +38,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [MethodImpl(MethodImplOptions.NoInlining)] private void UnrootedConnectionsGetRemovedFromHeartbeatInnerScope( string connectionId, - HttpConnectionManager httpConnectionManager, + ConnectionManager httpConnectionManager, Mock trace) { - var httpConnection = new HttpConnection(new HttpConnectionContext - { - ServiceContext = new TestServiceContext(), - ConnectionId = connectionId - }); + var mock = new Mock(); + mock.Setup(m => m.ConnectionId).Returns(connectionId); + var httpConnection = new KestrelConnection(mock.Object); httpConnectionManager.AddConnection(0, httpConnection); diff --git a/test/Kestrel.Core.Tests/HttpConnectionTests.cs b/test/Kestrel.Core.Tests/HttpConnectionTests.cs index 2c78c922c7..8d0f1e418b 100644 --- a/test/Kestrel.Core.Tests/HttpConnectionTests.cs +++ b/test/Kestrel.Core.Tests/HttpConnectionTests.cs @@ -43,7 +43,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests ConnectionAdapters = new List(), ConnectionFeatures = connectionFeatures, MemoryPool = _memoryPool, - HttpConnectionId = long.MinValue, Transport = pair.Transport, ServiceContext = new TestServiceContext { diff --git a/test/Kestrel.InMemory.FunctionalTests/HttpsTests.cs b/test/Kestrel.InMemory.FunctionalTests/HttpsTests.cs index 5d3ce03935..9e52de9f12 100644 --- a/test/Kestrel.InMemory.FunctionalTests/HttpsTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/HttpsTests.cs @@ -312,7 +312,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests LoggerFactory.AddProvider(loggerProvider); var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); var handshakeStartedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); TimeSpan handshakeTimeout = default; diff --git a/test/Kestrel.InMemory.FunctionalTests/KeepAliveTimeoutTests.cs b/test/Kestrel.InMemory.FunctionalTests/KeepAliveTimeoutTests.cs index 2cf8739ca4..716ff9583a 100644 --- a/test/Kestrel.InMemory.FunctionalTests/KeepAliveTimeoutTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/KeepAliveTimeoutTests.cs @@ -27,7 +27,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionClosedWhenKeepAliveTimeoutExpires() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -51,7 +51,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionKeptAliveBetweenRequests() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -76,7 +76,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionNotTimedOutWhileRequestBeingSent() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -113,7 +113,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests private async Task ConnectionNotTimedOutWhileAppIsRunning() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); var cts = new CancellationTokenSource(); using (var server = CreateServer(testContext, longRunningCt: cts.Token)) @@ -150,7 +150,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests private async Task ConnectionTimesOutWhenOpenedButNoRequestSent() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -167,7 +167,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests private async Task KeepAliveTimeoutDoesNotApplyToUpgradedConnections() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); var cts = new CancellationTokenSource(); using (var server = CreateServer(testContext, upgradeCt: cts.Token)) diff --git a/test/Kestrel.InMemory.FunctionalTests/RequestBodyTimeoutTests.cs b/test/Kestrel.InMemory.FunctionalTests/RequestBodyTimeoutTests.cs index e308b03a99..f8946d707e 100644 --- a/test/Kestrel.InMemory.FunctionalTests/RequestBodyTimeoutTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/RequestBodyTimeoutTests.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests { var gracePeriod = TimeSpan.FromSeconds(5); var serviceContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(serviceContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(serviceContext.ConnectionManager); var appRunningEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -136,7 +136,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests { var gracePeriod = TimeSpan.FromSeconds(5); var serviceContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(serviceContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(serviceContext.ConnectionManager); var appRunningTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var exceptionSwallowedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); diff --git a/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs b/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs index d6a924bf61..e5fd714179 100644 --- a/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs @@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionAbortedWhenRequestHeadersNotReceivedInTime(string headers) { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task RequestHeadersTimeoutCanceledAfterHeadersReceived() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -76,7 +76,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionAbortedWhenRequestLineNotReceivedInTime(string requestLine) { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -95,7 +95,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task TimeoutNotResetOnEachRequestLineCharacterReceived() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(testContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) diff --git a/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs b/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs index 07c4d642e3..34f6b92333 100644 --- a/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs @@ -877,7 +877,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests var appEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var delayEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var serviceContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HttpHeartbeatManager(serviceContext.ConnectionManager); + var heartbeatManager = new HeartbeatManager(serviceContext.ConnectionManager); using (var server = new TestServer(async context => { diff --git a/test/Kestrel.InMemory.FunctionalTests/UpgradeTests.cs b/test/Kestrel.InMemory.FunctionalTests/UpgradeTests.cs index 42f5659d5e..7dd73fe650 100644 --- a/test/Kestrel.InMemory.FunctionalTests/UpgradeTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/UpgradeTests.cs @@ -254,7 +254,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests const int limit = 10; var upgradeTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var serviceContext = new TestServiceContext(LoggerFactory); - serviceContext.ConnectionManager = new HttpConnectionManager(serviceContext.Log, ResourceCounter.Quota(limit)); + serviceContext.ConnectionManager = new ConnectionManager(serviceContext.Log, ResourceCounter.Quota(limit)); using (var server = new TestServer(async context => { diff --git a/test/Kestrel.Transport.FunctionalTests/Http2/ShutdownTests.cs b/test/Kestrel.Transport.FunctionalTests/Http2/ShutdownTests.cs index 6014dcc37d..64eceae6a5 100644 --- a/test/Kestrel.Transport.FunctionalTests/Http2/ShutdownTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/Http2/ShutdownTests.cs @@ -109,6 +109,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 MemoryPoolFactory = memoryPoolFactory.Create }; + TestApplicationErrorLogger.ThrowOnUngracefulShutdown = false; + // Abortive shutdown leaves one request hanging using (var server = new TestServer(async context => { diff --git a/test/shared/TestApplicationErrorLogger.cs b/test/shared/TestApplicationErrorLogger.cs index bff77f8efa..3f0f75f77b 100644 --- a/test/shared/TestApplicationErrorLogger.cs +++ b/test/shared/TestApplicationErrorLogger.cs @@ -20,6 +20,8 @@ namespace Microsoft.AspNetCore.Testing public bool ThrowOnCriticalErrors { get; set; } = true; + public bool ThrowOnUngracefulShutdown { get; set; } = true; + public ConcurrentQueue Messages { get; } = new ConcurrentQueue(); public ConcurrentQueue Scopes { get; } = new ConcurrentQueue(); @@ -59,7 +61,9 @@ namespace Microsoft.AspNetCore.Testing } // Fail tests where not all the connections close during server shutdown. - if (eventId.Id == 21 && eventId.Name == nameof(KestrelTrace.NotAllConnectionsAborted)) + if (ThrowOnUngracefulShutdown && + ((eventId.Id == 16 && eventId.Name == nameof(KestrelTrace.NotAllConnectionsClosedGracefully)) || + (eventId.Id == 21 && eventId.Name == nameof(KestrelTrace.NotAllConnectionsAborted)))) { var log = $"Log {logLevel}[{eventId}]: {formatter(state, exception)} {exception?.Message}"; throw new Exception($"Shutdown failure. {log}"); diff --git a/test/shared/TestServiceContext.cs b/test/shared/TestServiceContext.cs index f8b68dd906..356164e725 100644 --- a/test/shared/TestServiceContext.cs +++ b/test/shared/TestServiceContext.cs @@ -45,7 +45,7 @@ namespace Microsoft.AspNetCore.Testing SystemClock = new SystemClock(); DateHeaderValueManager = new DateHeaderValueManager(SystemClock); - var heartbeatManager = new HttpHeartbeatManager(ConnectionManager); + var heartbeatManager = new HeartbeatManager(ConnectionManager); Heartbeat = new Heartbeat( new IHeartbeatHandler[] { DateHeaderValueManager, heartbeatManager }, SystemClock, @@ -61,7 +61,7 @@ namespace Microsoft.AspNetCore.Testing MockSystemClock = new MockSystemClock(); SystemClock = MockSystemClock; DateHeaderValueManager = new DateHeaderValueManager(MockSystemClock); - ConnectionManager = new HttpConnectionManager(Log, ResourceCounter.Unlimited); + ConnectionManager = new ConnectionManager(Log, ResourceCounter.Unlimited); HttpParser = new HttpParser(Log.IsEnabled(LogLevel.Information)); ServerOptions = new KestrelServerOptions { diff --git a/tools/CodeGenerator/TransportConnectionFeatureCollection.cs b/tools/CodeGenerator/TransportConnectionFeatureCollection.cs index 9f19f5387f..350d853fe8 100644 --- a/tools/CodeGenerator/TransportConnectionFeatureCollection.cs +++ b/tools/CodeGenerator/TransportConnectionFeatureCollection.cs @@ -19,6 +19,8 @@ namespace CodeGenerator "IApplicationTransportFeature", "ITransportSchedulerFeature", "IConnectionLifetimeFeature", + "IConnectionHeartbeatFeature", + "IConnectionLifetimeNotificationFeature", "IBytesWrittenFeature", };