From 5a47aad754246bffc4aa655465f7091213eb9133 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 5 Dec 2018 15:43:31 -0800 Subject: [PATCH 1/4] Wait to dispose RequestAborted CTS --- .../Core/src/Internal/Http/HttpProtocol.cs | 36 +++++++++---------- .../Kestrel/Core/test/Http1ConnectionTests.cs | 22 +++++++++++- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 3309701e0a56..cf7ca09e9a96 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -348,11 +348,8 @@ public void Reset() // Lock to prevent CancelRequestAbortedToken from attempting to cancel an disposed CTS. lock (_abortLock) { - if (!_requestAborted) - { - _abortedCts?.Dispose(); - _abortedCts = null; - } + _abortedCts?.Dispose(); + _abortedCts = null; } _requestHeadersParsed = 0; @@ -394,15 +391,16 @@ protected virtual bool BeginRead(out ValueTask awaitable) private void CancelRequestAbortedToken() { - try - { - _abortedCts.Cancel(); - _abortedCts.Dispose(); - _abortedCts = null; - } - catch (Exception ex) + lock (_abortLock) { - Log.ApplicationError(ConnectionId, TraceIdentifier, ex); + try + { + _abortedCts?.Cancel(); + } + catch (Exception ex) + { + Log.ApplicationError(ConnectionId, TraceIdentifier, ex); + } } } @@ -416,12 +414,12 @@ protected void AbortRequest() } _requestAborted = true; - } - if (_abortedCts != null) - { - // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. - ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this); + if (_abortedCts != null && !_preventRequestAbortedCancellation) + { + // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. + ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this); + } } } @@ -441,8 +439,6 @@ private void PreventRequestAbortedCancellation() } _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 d60303d6250f..9c83e7456f62 100644 --- a/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs +++ b/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs @@ -53,7 +53,11 @@ public Http1ConnectionTests() var connectionFeatures = new FeatureCollection(); connectionFeatures.Set(Mock.Of()); - _serviceContext = new TestServiceContext(); + _serviceContext = new TestServiceContext() + { + Scheduler = PipeScheduler.Inline + }; + _timeoutControl = new Mock(); _http1ConnectionContext = new HttpConnectionContext { @@ -724,6 +728,22 @@ public async Task RequestAbortedTokenIsResetBeforeLastWriteWithChunkedEncoding() Assert.False(_http1Connection.RequestAborted.IsCancellationRequested); } + [Fact] + public void RequestAbortedTokenIsFullyUsableAfterCancellation() + { + var originalToken = _http1Connection.RequestAborted; + var originalRegistration = originalToken.Register(() => { }); + + _http1Connection.Abort(new ConnectionAbortedException()); + + Assert.True(originalToken.WaitHandle.WaitOne(TestConstants.DefaultTimeout)); + Assert.True(_http1Connection.RequestAborted.WaitHandle.WaitOne(TestConstants.DefaultTimeout)); + +#if NETCOREAPP2_2 + Assert.Equal(originalToken, originalRegistration.Token); +#endif + } + [Fact] public async Task ExceptionDetailNotIncludedWhenLogLevelInformationNotEnabled() { From 2159e4bb741ff94827c2953a1cc0422101b823e2 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 5 Dec 2018 15:59:44 -0800 Subject: [PATCH 2/4] Address PR feedback --- src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs b/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs index 9c83e7456f62..70301631ed4b 100644 --- a/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs +++ b/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs @@ -741,6 +741,9 @@ public void RequestAbortedTokenIsFullyUsableAfterCancellation() #if NETCOREAPP2_2 Assert.Equal(originalToken, originalRegistration.Token); +#elif NET461 +#else +#error Target framework needs to be updated #endif } From ae6eb890634e03b7d6084f2bc09b5ec0717bb5d8 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 5 Dec 2018 16:02:46 -0800 Subject: [PATCH 3/4] Add Kestrel.Core to PatchConfig.props --- eng/PatchConfig.props | 1 + 1 file changed, 1 insertion(+) diff --git a/eng/PatchConfig.props b/eng/PatchConfig.props index 42789ba1b130..22c8ab9dd190 100644 --- a/eng/PatchConfig.props +++ b/eng/PatchConfig.props @@ -20,6 +20,7 @@ Later on, this will be checked using this condition: Microsoft.AspNetCore.Server.IIS; Microsoft.AspNetCore.Server.IISIntegration; Microsoft.AspNetCore.Server.IntegrationTesting.IIS; + Microsoft.AspNetCore.Server.Kestrel.Core; Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets; Microsoft.AspNetCore.WebSockets; Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore; From 3753de765da4ae096b40bf4d90fb47c99b7c822d Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 14 Jan 2019 17:29:09 -0800 Subject: [PATCH 4/4] Move PackagesInPatch to 2.2.2 --- eng/PatchConfig.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/PatchConfig.props b/eng/PatchConfig.props index 22c8ab9dd190..f70184998004 100644 --- a/eng/PatchConfig.props +++ b/eng/PatchConfig.props @@ -20,7 +20,6 @@ Later on, this will be checked using this condition: Microsoft.AspNetCore.Server.IIS; Microsoft.AspNetCore.Server.IISIntegration; Microsoft.AspNetCore.Server.IntegrationTesting.IIS; - Microsoft.AspNetCore.Server.Kestrel.Core; Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets; Microsoft.AspNetCore.WebSockets; Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore; @@ -33,6 +32,7 @@ Later on, this will be checked using this condition: Microsoft.AspNetCore.Authentication.Google; Microsoft.AspNetCore.Http; Microsoft.AspNetCore.Server.IIS; + Microsoft.AspNetCore.Server.Kestrel.Core; java:signalr;