diff --git a/eng/PatchConfig.props b/eng/PatchConfig.props index 5076e60f99..b1b7e01318 100644 --- a/eng/PatchConfig.props +++ b/eng/PatchConfig.props @@ -26,7 +26,6 @@ Later on, this will be checked using this condition: Microsoft.AspNetCore.Mvc.Core; Microsoft.AspNetCore.Routing; Microsoft.AspNetCore.Server.IIS; - Microsoft.AspNetCore.Server.Kestrel.Core; java:signalr; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index e9b1d926aa..9d8dad0304 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -367,8 +367,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // Lock to prevent CancelRequestAbortedToken from attempting to cancel an disposed CTS. lock (_abortLock) { - _abortedCts?.Dispose(); - _abortedCts = null; + if (!_requestAborted) + { + _abortedCts?.Dispose(); + _abortedCts = null; + } } _requestHeadersParsed = 0; @@ -412,16 +415,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private void CancelRequestAbortedToken() { - lock (_abortLock) + try { - try - { - _abortedCts?.Cancel(); - } - catch (Exception ex) - { - Log.ApplicationError(ConnectionId, TraceIdentifier, ex); - } + _abortedCts.Cancel(); + _abortedCts.Dispose(); + _abortedCts = null; + } + catch (Exception ex) + { + Log.ApplicationError(ConnectionId, TraceIdentifier, ex); } } @@ -435,12 +437,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } _requestAborted = true; + } - if (_abortedCts != null && !_preventRequestAbortedCancellation) - { - // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. - ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this); - } + if (_abortedCts != null) + { + // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. + ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this); } } @@ -460,6 +462,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } _preventRequestAbortedCancellation = true; + _abortedCts?.Dispose(); + _abortedCts = null; } } diff --git a/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs b/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs index 96e230fd64..17263e2f6d 100644 --- a/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs +++ b/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs @@ -729,14 +729,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests } [Fact] - public void RequestAbortedTokenIsFullyUsableAfterCancellation() + public void RequestAbortedTokenIsUsableAfterCancellation() { var originalToken = _http1Connection.RequestAborted; var originalRegistration = originalToken.Register(() => { }); _http1Connection.Abort(new ConnectionAbortedException()); - Assert.True(originalToken.WaitHandle.WaitOne(TestConstants.DefaultTimeout)); + // The following line will throw an ODE because the original CTS backing the token has been diposed. + // See https://github.com/aspnet/AspNetCore/pull/4447 for the history behind this test. + //Assert.True(originalToken.WaitHandle.WaitOne(TestConstants.DefaultTimeout)); Assert.True(_http1Connection.RequestAborted.WaitHandle.WaitOne(TestConstants.DefaultTimeout)); Assert.Equal(originalToken, originalRegistration.Token);