Is it valid to use CancellationToken.Cancel() from a catch Exception block?

Question

Is it a valid use of CancellationToken.Cancel() to cancel other tasks running in parallel if one of the tasks throws an exception? For example:

using var tokenSource = new CancellationTokenSource();
var token = tokenSource.Token;

try
{
    var tasks = new List<Task>();
    tasks.Add(SomeDbContextBasedTaskAsync(token));
    ...
    tasks.Add(SomeDbContextBasedTaskAsync(token));
            
    Task.WaitAll(tasks.ToArray(), token);
}
catch (Exception ex)
{
    tokenSource.Cancel(); // Cancel any other tasks that are still in flight
}

Or is this an anti-pattern that could lead to issues? It seems a good application to me, but I haven't found any examples of the CancellationToken being used in this way, and a fellow developer I discussed it with felt that issues could occur applying the Cancel() from the catch block.

In my real-world application I'm making calls to DbContext based functionality that should pick up the CancellationToken and respond appropriately to it, rather than me needing to implement any hand-rolled cancellation code.

This question is subtly different to Cancel all async methods if one throws an exception as we're not await'ing every call in order. Here we're capturing a list of Task, all of which are in-flight when anyone of them may fail, throwing an exception. This list is only Task.WaitAll'd once all tasks are in-flight asynchronously.

The purpose of the Cancel() in this case it to prevent needless database calls from continuing after an exception has been detecting in any of the calls. Hopefully saving a bit of resource consumption.

Does this answer your question? [Cancel all async methods if one throws an exception](https://stackoverflow.com/questions/49347805/cancel-all-async-methods-if-one-throws-an-exception) — Maytham Fahmi, May 23 '22 at 03:46
`Cancel()` doesn't really do anything to tasks that are already in flight. It just flips a bit in a data structure. In order for anything to actually happen, your code has to do it (usually by checking `IsCancellationRequested` and doing something, or by calling `ThrowIfCancellationRequested`). So if calling `Cancel()` is going to cause issues, it will cause issues in *your* code. You didn't include any of that code in your question. So it's impossible to answer. — John Wu, May 23 '22 at 04:00
@JohnWu in my real world case the tasks are DbContext based tasks which should be handling the `Cancel()` internally. I'll update the question to make it a little more obvious. — Gavin, May 23 '22 at 04:41
It depends on your database tasks and how they handle cancellation. Sometimes things need to succeed or fail together, or fail in a certain way in order to maintain data consistency. But there is nothing inherently wrong about cancellation itself. — John Wu, May 24 '22 at 08:01
@JohnWu good point. In the real life scenario they're just independent health checks so are safe to cancel if one fails. — Gavin, May 25 '22 at 08:39

score 3 · Answer 1 · answered May 23 '22 at 06:08

3

Canceling tasks in an exception block should be fine.

But in your specific example it will not do anything. Task.WaitAll will wait untill all tasks has completed, meaning all the tasks has either reached RanToCompletion, Faulted, or Canceled. So canceling tasks that has already completed or failed will not do anything.

You probably want to trigger cancellation if any of the tasks fail, perhaps using something like an extension method:

public static async Task CancelOnException(this Task task, CancellationTokenSource cts)
{
    try
    {
        await task;
    }
    catch
    {
        cts.Cancel();
    }
}

However, cancellation will require your methods to actually be cancellable. Some database providers have limited or no support for cancellation. So I would verify that the operations can actually be cancelled in a timely manner before building anything complicated.

answered May 23 '22 at 06:08

JonasH

28,608
2
10
23

Thank you for the detailed explanation, much appreciated. Would it make any difference if instead of using `Task.WaitAll()` I used this approach instead? `Task task1 = SomeDbContextBasedTaskAsync(token); Task task2 = SomeDbContextBasedTaskAsync(token); await task1.Wait(); await task2.Wait();` - so if an exception occurs in `task1` there's a chance the `CancellationToken` may be picked up in task2 when I catch and cancel? More of an academic curiosity question now. – Gavin May 23 '22 at 21:12
@Gavin You do not do both await and wait, so I assume you mean `await task1; await task2`. If inside a try/catch this could allow cancellation of task2 if task1 fails. But not the reverse. You could use `WhenAny`. but that would only work for two tasks. – JonasH May 23 '22 at 21:23
My apologies, I meant `Task task1 = SomeDbContextBasedTaskAsync(token); Task task2 = SomeDbContextBasedTaskAsync(token); task1.Wait(); task2.Wait();` - allowing both tasks to be in-flight at the same time rather than awaiting each in turn synchronously. – Gavin May 23 '22 at 22:17
@Gavin There should be the same behavior both when waiting or awaiting with regards to exceptions, or at least similar enough for this context. – JonasH May 24 '22 at 06:24

Theodor Zoulias · Answer 2 · 2022-05-23T06:19:01.997

~~Yes, in general this pattern of using the CancellationTokenSource is OK~~¹. One gotcha that you should be aware of is that the Cancel method invokes all the callbacks that have been previously registered with the associated CancellationToken, on the current thread. So if each cancelable task has a synchronous continuation attached to it that does a small amount of CPU-bound work, and the tasks are many, the total amount of work for all these continuations might be significant. In this case the Cancel will effectively block the current thread until all the synchronous work has completed (another term is that the current thread has been temporarily "stolen"). Below is a minimal demonstration of this behavior, with only one task for simplicity:

var tokenSource = new CancellationTokenSource();
var task = Task.Delay(5000, tokenSource.Token);
_ = task.ContinueWith(_ => Thread.Sleep(1000), default,
    TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default);
Console.WriteLine($"Before Cancel: {DateTime.Now.TimeOfDay}");
tokenSource.Cancel();
Console.WriteLine($"After Cancel:  {DateTime.Now.TimeOfDay}");

Output:

Before Cancel: 06:02:54.8818432
After Cancel:  06:02:55.9011518

Try it on Fiddle.

You can avoid this by offloading the cancellation on a ThreadPool thread:

await Task.Run(() => tokenSource.Cancel());

This behavior might also cause reentrancy, in case you invoke the Cancel inside a lock-protected region, and another lock-protected region also exists in the continuations of the tasks.

¹ _{Actually it's not, see the answer by @JonasH.}

Jesús López · Answer 3 · 2022-05-23T07:44:31.147

As JonasH said you code doesn't work because Task.WaitAll waits for all of tasks to complete execution

However, if you want to cancel all tasks when any of them fails, you can use the following extension method:

public static void CancellAllWhenOneFails(this IEnumerable<Task> tasks, CancellationTokenSource cancellationTokenSource)
{
    if (tasks == null) throw new ArgumentNullException(nameof(tasks));
    foreach (var task in tasks)
    {
        task.ContinueWith(_ => cancellationTokenSource.Cancel(), 
            CancellationToken.None, TaskContinuationOptions.OnlyOnFaulted, TaskScheduler.Default);
    }
}

Your code would be:

using var tokenSource = new CancellationTokenSource();
var token = tokenSource.Token;

try
{
    var tasks = new List<Task>();
    tasks.Add(SomeDbContextBasedTaskAsync(token));
    tasks.Add(SomeDbContextBasedTaskAsync(token));
    tasks.CancellAllWhenOneFails(tokenSource);
    Task.WaitAll(tasks.ToArray());
}
catch (Exception ex)
{
    // if any of the tasks failed the rest are cancelled
    
}

Is it valid to use CancellationToken.Cancel() from a catch Exception block?

3 Answers3