Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid CancellationToken related allocations #787

Merged
merged 1 commit into from
Feb 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ internal abstract class ServerCallDeadlineManager : IAsyncDisposable
// Lock is to ensure deadline doesn't execute as call is completing
internal SemaphoreSlim Lock { get; }
// Internal for testing
internal bool _callComplete;
public bool CallComplete { get; private set; }

public CancellationToken CancellationToken => _deadlineCts!.Token;

Expand Down Expand Up @@ -79,16 +79,16 @@ public void Initialize(ISystemClock clock, TimeSpan timeout, CancellationToken r
_requestAbortedRegistration = requestAborted.Register(() =>
{
// Call is complete if the request has aborted
_callComplete = true;
CallComplete = true;
_deadlineCts?.Cancel();
});
}

protected abstract CancellationTokenSource CreateCancellationTokenSource(TimeSpan timeout, ISystemClock clock);

public void SetCallComplete()
public void SetCallEnded()
{
_callComplete = true;
CallComplete = true;
}

protected void DeadlineExceeded()
Expand Down Expand Up @@ -116,6 +116,7 @@ private async Task DeadlineExceededAsync()
}

await ServerCallContext.DeadlineExceededAsync();
CallComplete = true;
}
finally
{
Expand All @@ -127,7 +128,7 @@ private bool CanExceedDeadline()
{
// Deadline callback could be raised by the CTS after call has been completed (either successfully, with error, or aborted)
// but before deadline exceeded registration has been disposed
return !_callComplete;
return !CallComplete;
}

public ValueTask DisposeAsync()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,14 @@ private void ProcessHandlerError(Exception ex, string method)
}

// Don't update trailers if request has exceeded deadline/aborted
if (!CancellationToken.IsCancellationRequested)
if (DeadlineManager == null || !DeadlineManager.CallComplete)
{
HttpContext.Response.ConsolidateTrailers(this);
}

LogCallEnd();
DeadlineManager?.SetCallEnded();

DeadlineManager?.SetCallComplete();
LogCallEnd();
}

// If there is a deadline then we need to have our own cancellation token.
Expand Down Expand Up @@ -266,14 +266,14 @@ private async Task EndCallAsyncCore(Task lockTask)
private void EndCallCore()
{
// Don't set trailers if deadline exceeded or request aborted
if (!CancellationToken.IsCancellationRequested)
if (DeadlineManager == null || !DeadlineManager.CallComplete)
{
HttpContext.Response.ConsolidateTrailers(this);
}

LogCallEnd();
DeadlineManager?.SetCallEnded();

DeadlineManager?.SetCallComplete();
LogCallEnd();
}

private void LogCallEnd()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -608,15 +608,15 @@ private async Task LongRunningDeadline_WaitsUntilDeadlineIsFinished(string metho
Assert.Fail($"{methodName} did not wait on lock taken by deadline cancellation.");
}

Assert.IsFalse(serverCallContext.DeadlineManager!._callComplete);
Assert.IsFalse(serverCallContext.DeadlineManager!.CallComplete);

// Wait for dispose to finish
syncPoint.Continue();
await methodTask.DefaultTimeout();

Assert.AreEqual(GrpcProtocolConstants.ResetStreamNoError, httpResetFeature.ErrorCode);

Assert.IsTrue(serverCallContext.DeadlineManager!._callComplete);
Assert.IsTrue(serverCallContext.DeadlineManager!.CallComplete);
}

[Test]
Expand Down