Skip to content

Commit

Permalink
Avoid CancellationToken related allocations (#787)
Browse files Browse the repository at this point in the history
  • Loading branch information
JamesNK authored Feb 24, 2020
1 parent 2cb89bf commit 1159126
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 13 deletions.
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

0 comments on commit 1159126

Please sign in to comment.