diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 0503c549305b..8131508a82a1 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -35,7 +35,7 @@ internal abstract partial class HttpProtocol : IDefaultHttpContextContainer, IHt private Stack, object>> _onCompleted; private object _abortLock = new object(); - private volatile bool _requestAborted; + private volatile bool _connectionAborted; private bool _preventRequestAbortedCancellation; private CancellationTokenSource _abortedCts; private CancellationToken? _manuallySetRequestAbortToken; @@ -257,7 +257,7 @@ public CancellationToken RequestAborted return new CancellationToken(false); } - if (_requestAborted) + if (_connectionAborted) { return new CancellationToken(true); } @@ -375,18 +375,19 @@ public void Reset() Scheme = _scheme; _manuallySetRequestAbortToken = null; - _preventRequestAbortedCancellation = false; - // Lock to prevent CancelRequestAbortedToken from attempting to cancel an disposed CTS. + // Lock to prevent CancelRequestAbortedToken from attempting to cancel a disposed CTS. + CancellationTokenSource localAbortCts = null; + lock (_abortLock) { - if (!_requestAborted) - { - _abortedCts?.Dispose(); - _abortedCts = null; - } + _preventRequestAbortedCancellation = false; + localAbortCts = _abortedCts; + _abortedCts = null; } + localAbortCts?.Dispose(); + if (_wrapperObjectsToDispose != null) { foreach (var disposable in _wrapperObjectsToDispose) @@ -442,9 +443,20 @@ private void CancelRequestAbortedToken() { try { - _abortedCts.Cancel(); - _abortedCts.Dispose(); - _abortedCts = null; + CancellationTokenSource localAbortCts = null; + + lock (_abortLock) + { + if (_abortedCts != null && !_preventRequestAbortedCancellation) + { + localAbortCts = _abortedCts; + _abortedCts = null; + } + } + + // If we cancel the cts, we don't dispose as people may still be using + // the cts. It also isn't necessary to dispose a canceled cts. + localAbortCts?.Cancel(); } catch (Exception ex) { @@ -454,17 +466,20 @@ private void CancelRequestAbortedToken() protected void AbortRequest() { + var shouldScheduleCancellation = false; + lock (_abortLock) { - if (_requestAborted) + if (_connectionAborted) { return; } - _requestAborted = true; + shouldScheduleCancellation = _abortedCts != null && !_preventRequestAbortedCancellation; + _connectionAborted = true; } - if (_abortedCts != null) + if (shouldScheduleCancellation) { // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this); @@ -481,14 +496,12 @@ private void PreventRequestAbortedCancellation() { lock (_abortLock) { - if (_requestAborted) + if (_connectionAborted) { return; } _preventRequestAbortedCancellation = true; - _abortedCts?.Dispose(); - _abortedCts = null; } } @@ -594,7 +607,7 @@ private async Task ProcessRequests(IHttpApplication applicat // Run the application code for this request await application.ProcessRequestAsync(context); - if (!_requestAborted) + if (!_connectionAborted) { VerifyResponseContentLength(); } @@ -630,7 +643,7 @@ private async Task ProcessRequests(IHttpApplication applicat // 4XX responses are written by TryProduceInvalidRequestResponse during connection tear down. if (_requestRejectedException == null) { - if (!_requestAborted) + if (!_connectionAborted) { // Call ProduceEnd() before consuming the rest of the request body to prevent // delaying clients waiting for the chunk terminator: @@ -662,7 +675,7 @@ private async Task ProcessRequests(IHttpApplication applicat application.DisposeContext(context, _applicationException); // Even for non-keep-alive requests, try to consume the entire body to avoid RSTs. - if (!_requestAborted && _requestRejectedException == null && !messageBody.IsEmpty) + if (!_connectionAborted && _requestRejectedException == null && !messageBody.IsEmpty) { await messageBody.ConsumeAsync(); } @@ -964,7 +977,7 @@ private void VerifyInitializeState(int firstWriteByteCount) protected Task TryProduceInvalidRequestResponse() { // If _requestAborted is set, the connection has already been closed. - if (_requestRejectedException != null && !_requestAborted) + if (_requestRejectedException != null && !_connectionAborted) { return ProduceEnd(); } diff --git a/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs b/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs index ce1f86e7a6d5..6ac94c384a70 100644 --- a/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs +++ b/src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs @@ -729,6 +729,20 @@ 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)); + + Assert.Equal(originalToken, originalRegistration.Token); + } + [Fact] public void RequestAbortedTokenIsUsableAfterCancellation() { diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsTests.cs index 6f834ebc8ccd..efd1d567223a 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsTests.cs @@ -217,6 +217,7 @@ await sslStream.AuthenticateAsClientAsync("127.0.0.1", clientCertificates: null, } [Fact] + [Repeat(20)] public async Task DoesNotThrowObjectDisposedExceptionFromWriteAsyncAfterConnectionIsAborted() { var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);