diff options
author | Murat Girgin <murat@girg.in> | 2019-01-25 01:46:35 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-01-25 01:46:35 +0300 |
commit | 522705f9a27b99ca4ad261f2e89fe51a77b2338e (patch) | |
tree | 8a9052612b76b9458ddb7c996e93fa0ddd2c2f76 | |
parent | f4c5ac7635fb228693ff4c003890ba67510669ec (diff) | |
parent | 2853b451a2e689a540f97349bdad7b2251c8d729 (diff) |
Merge pull request #6994 from aspnet/halter73/1531-part2v2.2.2
Revert "Wait to dispose RequestAborted CTS (#4447)"
-rw-r--r-- | eng/PatchConfig.props | 1 | ||||
-rw-r--r-- | src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs | 36 | ||||
-rw-r--r-- | src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs | 6 |
3 files changed, 24 insertions, 19 deletions
diff --git a/eng/PatchConfig.props b/eng/PatchConfig.props index 98d068b407..7e408b0dcc 100644 --- a/eng/PatchConfig.props +++ b/eng/PatchConfig.props @@ -36,7 +36,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; </PackagesInPatch> </PropertyGroup> diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index cf7ca09e9a..3309701e0a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -348,8 +348,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; @@ -391,16 +394,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); } } @@ -414,12 +416,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); } } @@ -439,6 +441,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 70301631ed..b8420c3d5e 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)); #if NETCOREAPP2_2 |