huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Wait to dispose RequestAborted CTS (#9333)

This commit is contained in:
Justin Kotalik 2019-04-12 16:30:26 -07:00 committed by GitHub
parent 0684498efc
commit e04c79bd3d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 50 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs
View File

@ -35,7 +35,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
private Stack<KeyValuePair<Func<object, Task>, object>> _onCompleted; private Stack<KeyValuePair<Func<object, Task>, object>> _onCompleted;
private object _abortLock = new object(); private object _abortLock = new object();
private volatile bool _requestAborted; private volatile bool _connectionAborted;
private bool _preventRequestAbortedCancellation; private bool _preventRequestAbortedCancellation;
private CancellationTokenSource _abortedCts; private CancellationTokenSource _abortedCts;
private CancellationToken? _manuallySetRequestAbortToken; private CancellationToken? _manuallySetRequestAbortToken;
@ -257,7 +257,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
return new CancellationToken(false); return new CancellationToken(false);
} }
if (_requestAborted) if (_connectionAborted)
{ {
return new CancellationToken(true); return new CancellationToken(true);
} }
@ -375,18 +375,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
Scheme = _scheme; Scheme = _scheme;
_manuallySetRequestAbortToken = null; _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) lock (_abortLock)
{ {
if (!_requestAborted) _preventRequestAbortedCancellation = false;
{ localAbortCts = _abortedCts;
_abortedCts?.Dispose(); _abortedCts = null;
_abortedCts = null;
}
} }
localAbortCts?.Dispose();
if (_wrapperObjectsToDispose != null) if (_wrapperObjectsToDispose != null)
{ {
foreach (var disposable in _wrapperObjectsToDispose) foreach (var disposable in _wrapperObjectsToDispose)
@ -442,9 +443,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
{ {
try try
{ {
_abortedCts.Cancel(); CancellationTokenSource localAbortCts = null;
_abortedCts.Dispose();
_abortedCts = 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) catch (Exception ex)
{ {
@ -454,17 +466,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
protected void AbortRequest() protected void AbortRequest()
{ {
var shouldScheduleCancellation = false;
lock (_abortLock) lock (_abortLock)
{ {
if (_requestAborted) if (_connectionAborted)
{ {
return; return;
} }
_requestAborted = true; shouldScheduleCancellation = _abortedCts != null && !_preventRequestAbortedCancellation;
_connectionAborted = true;
} }
if (_abortedCts != null) if (shouldScheduleCancellation)
{ {
// Potentially calling user code. CancelRequestAbortedToken logs any exceptions. // Potentially calling user code. CancelRequestAbortedToken logs any exceptions.
ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this); ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this);
@ -481,14 +496,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
{ {
lock (_abortLock) lock (_abortLock)
{ {
if (_requestAborted) if (_connectionAborted)
{ {
return; return;
} }
_preventRequestAbortedCancellation = true; _preventRequestAbortedCancellation = true;
_abortedCts?.Dispose();
_abortedCts = null;
} }
} }
@ -594,7 +607,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
// Run the application code for this request // Run the application code for this request
await application.ProcessRequestAsync(context); await application.ProcessRequestAsync(context);
if (!_requestAborted) if (!_connectionAborted)
{ {
VerifyResponseContentLength(); VerifyResponseContentLength();
} }
@ -630,7 +643,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
// 4XX responses are written by TryProduceInvalidRequestResponse during connection tear down. // 4XX responses are written by TryProduceInvalidRequestResponse during connection tear down.
if (_requestRejectedException == null) if (_requestRejectedException == null)
{ {
if (!_requestAborted) if (!_connectionAborted)
{ {
// Call ProduceEnd() before consuming the rest of the request body to prevent // Call ProduceEnd() before consuming the rest of the request body to prevent
// delaying clients waiting for the chunk terminator: // delaying clients waiting for the chunk terminator:
@ -662,7 +675,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
application.DisposeContext(context, _applicationException); application.DisposeContext(context, _applicationException);
// Even for non-keep-alive requests, try to consume the entire body to avoid RSTs. // 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(); await messageBody.ConsumeAsync();
} }
@ -964,7 +977,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
protected Task TryProduceInvalidRequestResponse() protected Task TryProduceInvalidRequestResponse()
{ {
// If _requestAborted is set, the connection has already been closed. // If _requestAborted is set, the connection has already been closed.
if (_requestRejectedException != null && !_requestAborted) if (_requestRejectedException != null && !_connectionAborted)
{ {
return ProduceEnd(); return ProduceEnd();
} }

14
src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs
View File

@ -729,6 +729,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
Assert.False(_http1Connection.RequestAborted.IsCancellationRequested); 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] [Fact]
public void RequestAbortedTokenIsUsableAfterCancellation() public void RequestAbortedTokenIsUsableAfterCancellation()
{ {

1
src/Servers/Kestrel/test/InMemory.FunctionalTests/HttpsTests.cs
View File

@ -217,6 +217,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests
} }
[Fact] [Fact]
[Repeat(20)]
public async Task DoesNotThrowObjectDisposedExceptionFromWriteAsyncAfterConnectionIsAborted() public async Task DoesNotThrowObjectDisposedExceptionFromWriteAsyncAfterConnectionIsAborted()
{ {
var tcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously); var tcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);