Skip to content

Use ThrowIfCancellationRequested instead of simply returning #5343

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
@@ -109,10 +109,7 @@ public Task<bool> IsEnabledAsync()

public async Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested)
{
return;
}
cancellationToken.ThrowIfCancellationRequested();

if (value is not TestNodeUpdateMessage nodeUpdateMessage)
{
Original file line number Diff line number Diff line change
@@ -51,12 +51,12 @@ public Task<bool> IsEnabledAsync()

public Task BeforeTestHostProcessStartAsync(CancellationToken _) => Task.CompletedTask;

public Task OnTestHostProcessStartedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellation) => Task.CompletedTask;
public Task OnTestHostProcessStartedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellationToken) => Task.CompletedTask;

public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellation)
public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellationToken)
{
if (cancellation.IsCancellationRequested
|| testHostProcessInformation.HasExitedGracefully
cancellationToken.ThrowIfCancellationRequested();
if (testHostProcessInformation.HasExitedGracefully
|| (AppDomain.CurrentDomain.GetData("ProcessKilledByHangDump") is string processKilledByHangDump && processKilledByHangDump == "true"))
{
return;
Original file line number Diff line number Diff line change
@@ -89,8 +89,9 @@ public Task<bool> IsEnabledAsync() => Task.FromResult(_commandLineOptions.IsOpti
public async Task OnTestSessionStartingAsync(SessionUid sessionUid, CancellationToken cancellationToken)
{
ApplicationStateGuard.Ensure(_namedPipeClient is not null);
cancellationToken.ThrowIfCancellationRequested();

if (!await IsEnabledAsync() || cancellationToken.IsCancellationRequested)
if (!await IsEnabledAsync())
{
return;
}
@@ -156,8 +157,8 @@ private async Task<IResponse> CallbackAsync(IRequest request)

public async Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested
|| value is not TestNodeUpdateMessage nodeChangedMessage)
cancellationToken.ThrowIfCancellationRequested();
if (value is not TestNodeUpdateMessage nodeChangedMessage)
{
return;
}
Original file line number Diff line number Diff line change
@@ -173,7 +173,7 @@ private async Task<IResponse> CallbackAsync(IRequest request)
}
}

public async Task OnTestHostProcessStartedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellation)
public async Task OnTestHostProcessStartedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellationToken)
{
ApplicationStateGuard.Ensure(_waitConnectionTask is not null);
ApplicationStateGuard.Ensure(_singleConnectionNamedPipeServer is not null);
@@ -184,10 +184,10 @@ public async Task OnTestHostProcessStartedAsync(ITestHostProcessInformation test
await _logger.LogDebugAsync($"Wait for test host connection to the server pipe '{_singleConnectionNamedPipeServer.PipeName.Name}'");
await _waitConnectionTask.TimeoutAfterAsync(TimeoutHelper.DefaultHangTimeSpanTimeout);
using CancellationTokenSource timeout = new(TimeoutHelper.DefaultHangTimeSpanTimeout);
using var linkedCancellationToken = CancellationTokenSource.CreateLinkedTokenSource(cancellation, timeout.Token);
using var linkedCancellationToken = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeout.Token);
_waitConsumerPipeName.Wait(linkedCancellationToken.Token);
ApplicationStateGuard.Ensure(_namedPipeClient is not null);
await _namedPipeClient.ConnectAsync(cancellation).TimeoutAfterAsync(TimeoutHelper.DefaultHangTimeSpanTimeout);
await _namedPipeClient.ConnectAsync(cancellationToken).TimeoutAfterAsync(TimeoutHelper.DefaultHangTimeSpanTimeout);
await _logger.LogDebugAsync($"Connected to the test host server pipe '{_namedPipeClient.PipeName}'");

// Keep the custom thread to avoid to waste one from thread pool.
@@ -203,9 +203,9 @@ public async Task OnTestHostProcessStartedAsync(ITestHostProcessInformation test
await _outputDisplay.DisplayAsync(this, new ErrorMessageOutputDeviceData(string.Format(CultureInfo.InvariantCulture, ExtensionResources.HangDumpFailed, e.ToString(), GetDiskInfo())));
throw;
}
}, "[HangDump] ActivityTimerAsync", cancellation);
}, "[HangDump] ActivityTimerAsync", cancellationToken);
}
catch (OperationCanceledException) when (cancellation.IsCancellationRequested)
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
{
}
}
@@ -229,12 +229,9 @@ private static string GetDiskInfo()
return builder.ToString();
}

public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellation)
public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellationToken)
{
if (cancellation.IsCancellationRequested)
{
return;
}
cancellationToken.ThrowIfCancellationRequested();

if (!testHostProcessInformation.HasExitedGracefully)
{
Original file line number Diff line number Diff line change
@@ -61,10 +61,7 @@ public Task OnTestSessionFinishingAsync(SessionUid sessionUid, CancellationToken

public async Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested)
{
return;
}
cancellationToken.ThrowIfCancellationRequested();

// Avoid processing messages if the session has ended.
if (_sessionEnded)
Original file line number Diff line number Diff line change
@@ -118,10 +118,7 @@ public AppInsightsProvider(
// Initialize the telemetry client and start ingesting events.
private async Task IngestLoopAsync()
{
if (_testApplicationCancellationTokenSource.CancellationToken.IsCancellationRequested)
{
return;
}
_testApplicationCancellationTokenSource.CancellationToken.ThrowIfCancellationRequested();

try
{
Original file line number Diff line number Diff line change
@@ -105,7 +105,8 @@ public TrxReportGenerator(

public Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationToken cancellationToken)
{
if (!_isEnabled || cancellationToken.IsCancellationRequested)
cancellationToken.ThrowIfCancellationRequested();
if (!_isEnabled)
{
return Task.CompletedTask;
}
@@ -168,7 +169,8 @@ public Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationTo

public async Task OnTestSessionStartingAsync(SessionUid _, CancellationToken cancellationToken)
{
if (!_isEnabled || cancellationToken.IsCancellationRequested)
cancellationToken.ThrowIfCancellationRequested();
if (!_isEnabled)
{
return;
}
@@ -211,7 +213,8 @@ await _trxTestApplicationLifecycleCallbacks.NamedPipeClient.RequestReplyAsync<Te

public async Task OnTestSessionFinishingAsync(SessionUid sessionUid, CancellationToken cancellationToken)
{
if (!_isEnabled || cancellationToken.IsCancellationRequested)
cancellationToken.ThrowIfCancellationRequested();
if (!_isEnabled)
{
return;
}
Original file line number Diff line number Diff line change
@@ -137,12 +137,9 @@ public Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationTo
return Task.CompletedTask;
}

public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellation)
public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellationToken)
{
if (cancellation.IsCancellationRequested)
{
return;
}
cancellationToken.ThrowIfCancellationRequested();

Dictionary<IExtension, List<SessionFileArtifact>> artifacts = [];

@@ -168,7 +165,7 @@ public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testH
new TestAdapterInfo(_testAdapterInformationRequest!.TestAdapterId, _testAdapterInformationRequest.TestAdapterVersion),
_startTime,
testHostProcessInformation.ExitCode,
cancellation);
cancellationToken);

(string fileName, string? warning) = await trxReportGeneratorEngine.GenerateReportAsync(
isTestHostCrashed: true,
@@ -204,7 +201,7 @@ await _messageBus.PublishAsync(
new TestAdapterInfo(_testAdapterInformationRequest!.TestAdapterId, _testAdapterInformationRequest.TestAdapterVersion),
_startTime,
testHostProcessInformation.ExitCode,
cancellation);
cancellationToken);

await trxReportGeneratorEngine.AddArtifactsAsync(trxFile, artifacts);
}
Original file line number Diff line number Diff line change
@@ -52,7 +52,9 @@ public TrxTestApplicationLifecycleCallbacks(

public async Task BeforeRunAsync(CancellationToken cancellationToken)
{
if (!_isEnabled || cancellationToken.IsCancellationRequested)
cancellationToken.ThrowIfCancellationRequested();

if (!_isEnabled)
{
return;
}
Original file line number Diff line number Diff line change
@@ -89,10 +89,7 @@ public override async Task PublishAsync(IDataProducer dataProducer, IData data)
throw new InvalidOperationException("The message bus has not been built yet.");
}

if (_testApplicationCancellationTokenSource.CancellationToken.IsCancellationRequested)
{
return;
}
_testApplicationCancellationTokenSource.CancellationToken.ThrowIfCancellationRequested();

if (_isTraceLoggingEnabled)
{
@@ -141,10 +138,7 @@ public override async Task DrainDataAsync()
CancellationToken cancellationToken = _testApplicationCancellationTokenSource.CancellationToken;
while (anotherRound)
{
if (cancellationToken.IsCancellationRequested)
{
return;
}
cancellationToken.ThrowIfCancellationRequested();

if (totalNumberOfDrainAttempt == 0)
{
Original file line number Diff line number Diff line change
@@ -131,10 +131,7 @@ public async Task<long> DrainDataAsync()
while (Interlocked.CompareExchange(ref _totalPayloadReceived, totalPayloadReceived, totalPayloadProcessed) != totalPayloadProcessed)
{
// When we cancel we throw inside ConsumeAsync and we won't drain anymore any data
if (_cancellationToken.IsCancellationRequested)
{
break;
}
_cancellationToken.ThrowIfCancellationRequested();

await _task.Delay(currentDelayTimeMs);
currentDelayTimeMs = Math.Min(currentDelayTimeMs + minDelayTimeMs, 200);
Original file line number Diff line number Diff line change
@@ -120,10 +120,7 @@ public async Task<long> DrainDataAsync()
while (Interlocked.CompareExchange(ref _totalPayloadReceived, totalPayloadReceived, totalPayloadProcessed) != totalPayloadProcessed)
{
// When we cancel we throw inside ConsumeAsync and we won't drain anymore any data
if (_cancellationToken.IsCancellationRequested)
{
break;
}
_cancellationToken.ThrowIfCancellationRequested();

await _task.Delay(currentDelayTimeMs);
currentDelayTimeMs = Math.Min(currentDelayTimeMs + minDelayTimeMs, 200);
Original file line number Diff line number Diff line change
@@ -234,10 +234,7 @@ public async Task DisplayAfterSessionEndRunAsync()

public Task OnTestSessionStartingAsync(SessionUid sessionUid, CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested)
{
return Task.CompletedTask;
}
cancellationToken.ThrowIfCancellationRequested();

// We implement IDataConsumerService and IOutputDisplayService.
// So the engine is calling us before as IDataConsumerService and after as IOutputDisplayService.
@@ -328,10 +325,7 @@ private void OnFailedTest(TestNodeUpdateMessage testNodeStateChanged, TestNodeSt

public Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested)
{
return Task.CompletedTask;
}
cancellationToken.ThrowIfCancellationRequested();

switch (value)
{
Original file line number Diff line number Diff line change
@@ -322,7 +322,8 @@ private async Task DisplayAfterSessionEndRunInternalAsync()

public Task OnTestSessionStartingAsync(SessionUid sessionUid, CancellationToken cancellationToken)
{
if (_isServerMode || cancellationToken.IsCancellationRequested)
cancellationToken.ThrowIfCancellationRequested();
if (_isServerMode)
{
return Task.CompletedTask;
}
@@ -382,8 +383,8 @@ public async Task DisplayAsync(IOutputDeviceDataProducer producer, IOutputDevice
public Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationToken cancellationToken)
{
RoslynDebug.Assert(_terminalTestReporter is not null);

if (_isServerMode || cancellationToken.IsCancellationRequested)
cancellationToken.ThrowIfCancellationRequested();
if (_isServerMode)
{
return Task.CompletedTask;
}
Original file line number Diff line number Diff line change
@@ -85,10 +85,7 @@ public TestNodeStatistics GetTestNodeStatistics()

private async Task ProcessTestNodeUpdateAsync(TestNodeUpdateMessage update, CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested)
{
return;
}
cancellationToken.ThrowIfCancellationRequested();

try
{
@@ -146,10 +143,7 @@ private async Task SendTestNodeUpdatesOnIdleAsync(Guid runId)
Guard.NotNull(_task);
await Task.WhenAny(_task.Delay(TimeSpan.FromMilliseconds(TestNodeUpdateDelayInMs), cancellationToken), _testSessionEnd.Task);

if (cancellationToken.IsCancellationRequested)
{
return;
}
cancellationToken.ThrowIfCancellationRequested();

await SendTestNodeUpdatesIfNecessaryAsync(runId, cancellationToken);
}
@@ -210,10 +204,7 @@ public async Task OnTestSessionFinishingAsync(SessionUid sessionUid, Cancellatio

public async Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested)
{
return;
}
cancellationToken.ThrowIfCancellationRequested();

switch (value)
{