Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Better reactive test cancellation
  • Loading branch information
thomhurst committed Dec 24, 2025
commit 3ec86372979dc6e295c78b5bb8dee03be8f7e4b8
7 changes: 7 additions & 0 deletions TUnit.Engine.Tests/CanCancelTests.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
using Shouldly;
using TUnit.Core.Enums;
using TUnit.Engine.Tests.Enums;

namespace TUnit.Engine.Tests;

/// <summary>
/// Tests that verify test cancellation works correctly when graceful cancellation is requested.
/// Skipped on Windows because CliWrap's graceful cancellation uses GenerateConsoleCtrlEvent,
/// which doesn't work properly for subprocess control.
/// </summary>
[ExcludeOn(OS.Windows)]
public class CanCancelTests(TestMode testMode) : InvokableTestBase(testMode)
{
[Test, Timeout(30_000)]
Expand Down
112 changes: 31 additions & 81 deletions TUnit.Engine/Helpers/TimeoutHelper.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
using System.Diagnostics.CodeAnalysis;

namespace TUnit.Engine.Helpers;

/// <summary>
Expand All @@ -11,103 +9,56 @@ internal static class TimeoutHelper
/// Executes a task with an optional timeout. If the timeout elapses before the task completes,
/// control is returned to the caller immediately with a TimeoutException.
/// </summary>
/// <param name="taskFactory">Factory function that creates the task to execute</param>
/// <param name="timeout">Optional timeout duration. If null, no timeout is applied</param>
/// <param name="cancellationToken">Cancellation token for the operation</param>
/// <param name="timeoutMessage">Optional custom timeout message. If null, uses default message</param>
/// <returns>The completed task</returns>
/// <exception cref="TimeoutException">Thrown when the timeout elapses before task completion</exception>
public static async Task ExecuteWithTimeoutAsync(
Func<CancellationToken, Task> taskFactory,
TimeSpan? timeout,
CancellationToken cancellationToken,
string? timeoutMessage = null)
Comment on lines 24 to 28
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important XML documentation for parameters and return values has been removed. According to TUnit development guidelines, code documentation should be maintained. The removed documentation described the parameters (taskFactory, timeout, cancellationToken, timeoutMessage) and return value, as well as the TimeoutException that can be thrown. Restore this documentation.

Copilot uses AI. Check for mistakes.
{
if (!timeout.HasValue)
{
await taskFactory(cancellationToken).ConfigureAwait(false);
return;
}

using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
timeoutCts.CancelAfter(timeout.Value);

var executionTask = taskFactory(timeoutCts.Token);

// Use a cancellable timeout task to avoid leaving Task.Delay running in the background
using var timeoutTaskCts = new CancellationTokenSource();
var timeoutTask = Task.Delay(timeout.Value, timeoutTaskCts.Token);

var completedTask = await Task.WhenAny(executionTask, timeoutTask).ConfigureAwait(false);

if (completedTask == timeoutTask)
{
// Timeout occurred - cancel the execution task and wait briefly for cleanup
timeoutCts.Cancel();

// Give the execution task a chance to handle cancellation gracefully
try
{
await executionTask.ConfigureAwait(false);
}
catch (OperationCanceledException)
{
// Expected when cancellation is properly handled
}
catch
await ExecuteWithTimeoutAsync(
async ct =>
{
// Ignore other exceptions from the cancelled task
}

var message = timeoutMessage ?? $"Operation timed out after {timeout.Value}";
throw new TimeoutException(message);
}

// Task completed normally - cancel the timeout task to free resources immediately
timeoutTaskCts.Cancel();

// Await the result to propagate any exceptions
await executionTask.ConfigureAwait(false);
await taskFactory(ct).ConfigureAwait(false);
return true;
},
timeout,
cancellationToken,
timeoutMessage).ConfigureAwait(false);
}

/// <summary>
/// Executes a task with an optional timeout and returns a result. If the timeout elapses before the task completes,
/// control is returned to the caller immediately with a TimeoutException.
/// </summary>
/// <typeparam name="T">The type of result returned by the task</typeparam>
/// <param name="taskFactory">Factory function that creates the task to execute</param>
/// <param name="timeout">Optional timeout duration. If null, no timeout is applied</param>
/// <param name="cancellationToken">Cancellation token for the operation</param>
/// <param name="timeoutMessage">Optional custom timeout message. If null, uses default message</param>
/// <returns>The result of the completed task</returns>
/// <exception cref="TimeoutException">Thrown when the timeout elapses before task completion</exception>
public static async Task<T> ExecuteWithTimeoutAsync<T>(
Func<CancellationToken, Task<T>> taskFactory,
TimeSpan? timeout,
CancellationToken cancellationToken,
string? timeoutMessage = null)
Comment on lines 53 to 57
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important XML documentation for the generic type parameter and method parameters has been removed. According to TUnit development guidelines, code documentation should be maintained. The removed documentation described the type parameter T and the parameters (taskFactory, timeout, cancellationToken, timeoutMessage), return value, and TimeoutException. Restore this documentation.

Copilot uses AI. Check for mistakes.
{
if (!timeout.HasValue)
{
return await taskFactory(cancellationToken).ConfigureAwait(false);
}
var executionTask = taskFactory(cancellationToken);
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking behavioral change from the previous implementation. The old code passed a linked cancellation token (timeoutCts.Token) to the taskFactory, which would be automatically cancelled when the timeout elapsed. The new code passes the original cancellationToken, which means the task won't be automatically cancelled on timeout. This could cause tasks to continue running in the background after timeout, leading to resource leaks and unexpected behavior. Restore the previous behavior by creating a linked CancellationTokenSource and passing its token to taskFactory.

Copilot uses AI. Check for mistakes.

using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
timeoutCts.CancelAfter(timeout.Value);
// Create a task that completes when cancellation is requested
var tcs = new TaskCompletionSource<bool>();
using var registration = cancellationToken.Register(() => tcs.TrySetResult(true));
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TaskCompletionSource and its cancellation token registration could cause a memory leak if the executionTask completes before cancellation. The registration is disposed when the using statement exits, but if executionTask completes first, the registration remains active until the method returns. Consider disposing the registration as soon as the executionTask completes to free resources immediately, or use a try-finally pattern to ensure proper cleanup.

Copilot uses AI. Check for mistakes.

var executionTask = taskFactory(timeoutCts.Token);

// Use a cancellable timeout task to avoid leaving Task.Delay running in the background
using var timeoutTaskCts = new CancellationTokenSource();
var timeoutTask = Task.Delay(timeout.Value, timeoutTaskCts.Token);
// Create timeout task if timeout is specified
using var timeoutCts = new CancellationTokenSource();
var timeoutTask = timeout.HasValue
? Task.Delay(timeout.Value, timeoutCts.Token)
: Task.Delay(Timeout.Infinite, timeoutCts.Token);

var completedTask = await Task.WhenAny(executionTask, timeoutTask).ConfigureAwait(false);
var winningTask = await Task.WhenAny(executionTask, tcs.Task, timeoutTask).ConfigureAwait(false);

if (completedTask == timeoutTask)
// Cancellation requested
if (winningTask == tcs.Task)
{
throw new OperationCanceledException(cancellationToken);
}

// Timeout occurred
if (winningTask == timeoutTask)
{
// Timeout occurred - cancel the execution task and wait briefly for cleanup
timeoutCts.Cancel();

// Give the execution task a chance to handle cancellation gracefully
try
{
Expand All @@ -121,15 +72,14 @@ public static async Task<T> ExecuteWithTimeoutAsync<T>(
{
// Ignore other exceptions from the cancelled task
}
var message = timeoutMessage ?? $"Operation timed out after {timeout.Value}";

var message = timeoutMessage ?? $"Operation timed out after {timeout!.Value}";
throw new TimeoutException(message);
}

// Task completed normally - cancel the timeout task to free resources immediately
timeoutTaskCts.Cancel();
// Task completed normally - cancel the timeout task to free resources
timeoutCts.Cancel();

// Await the result to propagate any exceptions
return await executionTask.ConfigureAwait(false);
}
}
}
Loading