Wait to dispose RequestAborted CTS (#4447)

This commit is contained in:
Stephen Halter 2019-01-15 14:54:10 -08:00 committed by GitHub
parent 180f735ac8
commit 0622513058
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 41 additions and 21 deletions

View File

@ -33,6 +33,7 @@ Later on, this will be checked using this condition:
Microsoft.AspNetCore.Http; Microsoft.AspNetCore.Http;
Microsoft.AspNetCore.Mvc.Core; Microsoft.AspNetCore.Mvc.Core;
Microsoft.AspNetCore.Server.IIS; Microsoft.AspNetCore.Server.IIS;
Microsoft.AspNetCore.Server.Kestrel.Core;
java:signalr; java:signalr;
</PackagesInPatch> </PackagesInPatch>
</PropertyGroup> </PropertyGroup>

View File

@ -347,13 +347,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
// Lock to prevent CancelRequestAbortedToken from attempting to cancel an disposed CTS. // Lock to prevent CancelRequestAbortedToken from attempting to cancel an disposed CTS.
lock (_abortLock) lock (_abortLock)
{
if (!_requestAborted)
{ {
_abortedCts?.Dispose(); _abortedCts?.Dispose();
_abortedCts = null; _abortedCts = null;
} }
}
_requestHeadersParsed = 0; _requestHeadersParsed = 0;
@ -393,18 +390,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
protected abstract bool TryParseRequest(ReadResult result, out bool endConnection); protected abstract bool TryParseRequest(ReadResult result, out bool endConnection);
private void CancelRequestAbortedToken() private void CancelRequestAbortedToken()
{
lock (_abortLock)
{ {
try try
{ {
_abortedCts.Cancel(); _abortedCts?.Cancel();
_abortedCts.Dispose();
_abortedCts = null;
} }
catch (Exception ex) catch (Exception ex)
{ {
Log.ApplicationError(ConnectionId, TraceIdentifier, ex); Log.ApplicationError(ConnectionId, TraceIdentifier, ex);
} }
} }
}
protected void AbortRequest() protected void AbortRequest()
{ {
@ -416,14 +414,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
} }
_requestAborted = true; _requestAborted = true;
}
if (_abortedCts != null) if (_abortedCts != null && !_preventRequestAbortedCancellation)
{ {
// 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);
} }
} }
}
protected void PoisonRequestBodyStream(Exception abortReason) protected void PoisonRequestBodyStream(Exception abortReason)
{ {
@ -441,8 +439,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
} }
_preventRequestAbortedCancellation = true; _preventRequestAbortedCancellation = true;
_abortedCts?.Dispose();
_abortedCts = null;
} }
} }

View File

@ -53,7 +53,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
var connectionFeatures = new FeatureCollection(); var connectionFeatures = new FeatureCollection();
connectionFeatures.Set(Mock.Of<IConnectionLifetimeFeature>()); connectionFeatures.Set(Mock.Of<IConnectionLifetimeFeature>());
_serviceContext = new TestServiceContext(); _serviceContext = new TestServiceContext()
{
Scheduler = PipeScheduler.Inline
};
_timeoutControl = new Mock<ITimeoutControl>(); _timeoutControl = new Mock<ITimeoutControl>();
_http1ConnectionContext = new HttpConnectionContext _http1ConnectionContext = new HttpConnectionContext
{ {
@ -724,6 +728,25 @@ 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));
#if NETCOREAPP2_2
Assert.Equal(originalToken, originalRegistration.Token);
#elif NET461
#else
#error Target framework needs to be updated
#endif
}
[Fact] [Fact]
public async Task ExceptionDetailNotIncludedWhenLogLevelInformationNotEnabled() public async Task ExceptionDetailNotIncludedWhenLogLevelInformationNotEnabled()
{ {