Skip to content

Commit c8bd94e

Browse files
committed
Revert "Revert "Wait to dispose RequestAborted CTS (#4447)""
This reverts commit 29b7c5c.
1 parent 5199cce commit c8bd94e

File tree

3 files changed

+36
-21
lines changed

3 files changed

+36
-21
lines changed

eng/PatchConfig.props

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ Later on, this will be checked using this condition:
2626
Microsoft.AspNetCore.Mvc.Core;
2727
Microsoft.AspNetCore.Routing;
2828
Microsoft.AspNetCore.Server.IIS;
29+
Microsoft.AspNetCore.Server.Kestrel.Core;
2930
java:signalr;
3031
</PackagesInPatch>
3132
</PropertyGroup>

src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -367,11 +367,8 @@ public void Reset()
367367
// Lock to prevent CancelRequestAbortedToken from attempting to cancel an disposed CTS.
368368
lock (_abortLock)
369369
{
370-
if (!_requestAborted)
371-
{
372-
_abortedCts?.Dispose();
373-
_abortedCts = null;
374-
}
370+
_abortedCts?.Dispose();
371+
_abortedCts = null;
375372
}
376373

377374
_requestHeadersParsed = 0;
@@ -415,15 +412,16 @@ protected virtual bool BeginRead(out ValueTask<ReadResult> awaitable)
415412

416413
private void CancelRequestAbortedToken()
417414
{
418-
try
419-
{
420-
_abortedCts.Cancel();
421-
_abortedCts.Dispose();
422-
_abortedCts = null;
423-
}
424-
catch (Exception ex)
415+
lock (_abortLock)
425416
{
426-
Log.ApplicationError(ConnectionId, TraceIdentifier, ex);
417+
try
418+
{
419+
_abortedCts?.Cancel();
420+
}
421+
catch (Exception ex)
422+
{
423+
Log.ApplicationError(ConnectionId, TraceIdentifier, ex);
424+
}
427425
}
428426
}
429427

@@ -437,12 +435,12 @@ protected void AbortRequest()
437435
}
438436

439437
_requestAborted = true;
440-
}
441438

442-
if (_abortedCts != null)
443-
{
444-
// Potentially calling user code. CancelRequestAbortedToken logs any exceptions.
445-
ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this);
439+
if (_abortedCts != null && !_preventRequestAbortedCancellation)
440+
{
441+
// Potentially calling user code. CancelRequestAbortedToken logs any exceptions.
442+
ServiceContext.Scheduler.Schedule(state => ((HttpProtocol)state).CancelRequestAbortedToken(), this);
443+
}
446444
}
447445
}
448446

@@ -462,8 +460,6 @@ private void PreventRequestAbortedCancellation()
462460
}
463461

464462
_preventRequestAbortedCancellation = true;
465-
_abortedCts?.Dispose();
466-
_abortedCts = null;
467463
}
468464
}
469465

src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ public Http1ConnectionTests()
5353
var connectionFeatures = new FeatureCollection();
5454
connectionFeatures.Set(Mock.Of<IConnectionLifetimeFeature>());
5555

56-
_serviceContext = new TestServiceContext();
56+
_serviceContext = new TestServiceContext()
57+
{
58+
Scheduler = PipeScheduler.Inline
59+
};
60+
5761
_timeoutControl = new Mock<ITimeoutControl>();
5862
_http1ConnectionContext = new HttpConnectionContext
5963
{
@@ -724,6 +728,20 @@ public async Task RequestAbortedTokenIsResetBeforeLastWriteWithChunkedEncoding()
724728
Assert.False(_http1Connection.RequestAborted.IsCancellationRequested);
725729
}
726730

731+
[Fact]
732+
public void RequestAbortedTokenIsFullyUsableAfterCancellation()
733+
{
734+
var originalToken = _http1Connection.RequestAborted;
735+
var originalRegistration = originalToken.Register(() => { });
736+
737+
_http1Connection.Abort(new ConnectionAbortedException());
738+
739+
Assert.True(originalToken.WaitHandle.WaitOne(TestConstants.DefaultTimeout));
740+
Assert.True(_http1Connection.RequestAborted.WaitHandle.WaitOne(TestConstants.DefaultTimeout));
741+
742+
Assert.Equal(originalToken, originalRegistration.Token);
743+
}
744+
727745
[Fact]
728746
public async Task ExceptionDetailNotIncludedWhenLogLevelInformationNotEnabled()
729747
{

0 commit comments

Comments
 (0)