Async Tasks, Cancellation, and Exceptions

Question

I'm currently learning how to properly expose asynchronous parts of our library API with Tasks so they can be easier and nicer to use for customers. I decided to go with the TaskCompletionSource approach of wrapping a Task around it that doesn't get scheduled on the thread pool (in the instance here unneeded anyway since it's basically just a timer). This works fine, but cancellation is a bit of a headache right now.

The example shows the basic usage, registering a delegate on the token, but is a tiny bit more complicated than in my case, more to the point, I'm not really sure what to do with the TaskCanceledException. The documentation says either just returning and having the task status switch to RanToCompletion, or throwing an OperationCanceledException (which results in the task's result being Canceled) is fine. However, the examples seem to only pertain to, or at least mention, tasks that are started via a delegate passed to TaskFactory.StartNew.

My code currently is (roughly) as follows:

public Task Run(IFoo foo, CancellationToken token = default(CancellationToken)) {
  var tcs = new TaskCompletionSource<object>();

  // Regular finish handler
  EventHandler<EventArgs> callback = (sender, args) => tcs.TrySetResult(null);
  // Cancellation
  token.Register(() => {
    tcs.TrySetCanceled();
    CancelAndCleanupFoo(foo);
  });

  RunFoo(foo, callback);
  return tcs.Task;
}

(There is no result and no possible exceptions during execution; one reason why I chose to start here, and not at more complicated places in the library.)

In the current form, when I call TrySetCanceled on the TaskCompletionSource, I always get a TaskCanceledException if I await the task returned. My guess would be that this is normal behaviour (I hope it is) and that I'm expected to wrap a try/catch around the call when I want to use cancellation.

If I don't use TrySetCanceled, then I'll eventually run in the finish callback and the task looks like it finished normally. But I guess if a user wants to distinguish between a task that finished normally and one that was canceled, the TaskCanceledException is pretty much a side-effect of ensuring that, right?

Another point I didn't quite understand: Documentation suggests that any exceptions, even those that pertain to cancellation, are wrapped in an AggregateException by the TPL. However, in my tests I'd always get the TaskCanceledException directly, without any wrapper. Am I missing something here, or is that just poorly-documented?

---

TL;DR:

• For a task to transition into the Canceled state, there's always a corresponding exception needed for that and users have to wrap a try/catch around the async call to be able to detect that, right?
• The TaskCanceledException being thrown unwrapped is also expected and normal and I'm not doing anything wrong here?

Answer 1

I always recommend people read the Cancellation in Managed Threads docs. It's not quite complete; like most MSDN docs, it tells you what you can do, not what you should do. But it's definitely more clear than the dotnet docs on cancellation.

The example shows the basic usage

First, it's important to note that the cancellation in your example code only cancels the task - it does not cancel the underlying operation. I strongly recommend that you do not do this.

If you want to cancel the operation, then you would need to update RunFoo to take a CancellationToken (see below for how it should use it):

public Task Run(IFoo foo, CancellationToken token = default(CancellationToken)) {
  var tcs = new TaskCompletionSource<object>();

  // Regular finish handler
  EventHandler<AsyncCompletedEventArgs> callback = (sender, args) =>
  {
    if (args.Cancelled)
    {
      tcs.TrySetCanceled(token);
      CleanupFoo(foo);
    }
    else
      tcs.TrySetResult(null);
  };

  RunFoo(foo, token, callback);
  return tcs.Task;
}

If you can't cancel foo, then don't have your API support cancellation at all:

public Task Run(IFoo foo) {
  var tcs = new TaskCompletionSource<object>();

  // Regular finish handler
  EventHandler<EventArgs> callback = (sender, args) => tcs.TrySetResult(null);

  RunFoo(foo, callback);
  return tcs.Task;
}

Callers can then perform a cancelable wait on the task, which is a much more appropriate code technique for this scenario (since it is the wait that is canceled, not the operation represented by the task). Performing a "cancelable wait" can be done via my AsyncEx.Tasks library, or you could write your own equivalent extension method.

The documentation says either just returning and having the task status switch to RanToCompletion, or throwing an OperationCanceledException (which results in the task's result being Canceled) is fine.

Yeah, those docs are misleading. First, please don't just return; your method would complete the task successfully - indicating the operation completed successfully - when in fact the operation did not complete successfully. This may work for some code, but is certainly not a good idea in general.

Normally, the proper way to respond to a CancellationToken is to either:

• Periodically call ThrowIfCancellationRequested. This option is better for CPU-bound code.
• Register a cancellation callback via Register. This option is better for I/O-bound code. Note that registrations must be disposed!

In your particular case, you have an unusual situation. In your case, I would take a third approach:

• In your "per-frame work", check token.IsCancellationRequested; if it is requested, then raise the callback event with AsyncCompletedEventArgs.Cancelled set to true.

This is logically equivalent to the first proper way (periodically calling ThrowIfCancellationRequested), catching the exception, and translating it into an event notification. Just without the exception.

I always get a TaskCanceledException if I await the task returned. My guess would be that this is normal behaviour (I hope it is) and that I'm expected to wrap a try/catch around the call when I want to use cancellation.

The proper consuming code for a task that can be canceled is to wrap the await in a try/catch and catch OperationCanceledException. For various reasons (many historical), some APIs will cause OperationCanceledException and some will cause TaskCanceledException. Since TaskCanceledException derives from OperationCanceledException, consuming code can just catch the more general exception.

But I guess if a user wants to distinguish between a task that finished normally and one that was canceled, the [cancellation exception] is pretty much a side-effect of ensuring that, right?

That's the accepted pattern, yes.

Documentation suggests that any exceptions, even those that pertain to cancellation, are wrapped in an AggregateException by the TPL.

This is only true if your code synchronously blocks on a task. Which it should really avoid doing in the first place. So the docs are definitely misleading again.

However, in my tests I'd always get the TaskCanceledException directly, without any wrapper.

await avoids the AggregateException wrapper.

Update for comment explaining CleanupFoo is a cancellation method.

I'd first recommend trying to use CancellationToken directly within the code initiated by RunFoo; that approach would almost certainly be easier.

However, if you must use CleanupFoo for cancellation, then you'll need to Register it. You'll need to dispose that registration, and the easiest way to do this may actually be to split it into two different methods:

private Task DoRun(IFoo foo) {
  var tcs = new TaskCompletionSource<object>();

  // Regular finish handler
  EventHandler<EventArgs> callback = (sender, args) => tcs.TrySetResult(null);

  RunFoo(foo, callback);
  return tcs.Task;
}

public async Task Run(IFoo foo, CancellationToken token = default(CancellationToken)) {
  var tcs = new TaskCompletionSource<object>();
  using (token.Register(() =>
      {
        tcs.TrySetCanceled(token);
        CleanupFoo();
      });
  {
    var task = DoRun(foo);
    try
    {
      await task;
      tcs.TrySetResult(null);
    }
    catch (Exception ex)
    {
      tcs.TrySetException(ex);
    }
  }
  await tcs.Task;
}

Properly coordinating and propagating the results - while preventing resource leaks - is quite awkward. If your code could use CancellationToken directly, it would be much cleaner.

Answer 2

From the comments, it looks like you have an animation library that accepts an IAnimation, executes it (obviously asynchronously) and then signals back that it completed.

This isn't an actual task, in the sense that it isn't a piece of work that has to run on a thread. It's an asynchronous operation, which in .NET is exposed using a Task object.

Furthermore, you aren't actually cancelling something, you are stopping the animation. That's a perfectly normal operation so it shouldn't throw an exception. It would be better if your method returned a value that explained whether the animation completed or not, eg:

public Task<bool> Run(IAnimation animation, CancellationToken token = default(CancellationToken)) {
  var tcs = new TaskCompletionSource<bool>();

  // Regular finish handler
  EventHandler<EventArgs> callback = (sender, args) => tcs.TrySetResult(true);
  // Cancellation 
  token.Register(() => {
                         CleanupFoo(animation);
                         tcs.TrySetResult(false);
                       });
  RunFoo(animation, callback);
  return tcs.Task;
}

The call to run an animation is simple:

var myAnimation = new SomeAnimation();
var completed = await runner.Run(myAnimation,token);
if (completed)
{
}

UPDATE

This can be improved further with some C# 7 tricks.

For example, instead of using callbacks and lambdas, you can use local functions. Apart from making the code cleaner, they don't allocate a delegate each time they are called. The change doesn't require C# 7 support on the client's side:

Task<bool> Run(IAnimation animation, CancellationToken token = default(CancellationToken)) {
  var tcs = new TaskCompletionSource<bool>();

  // Regular finish handler
  void OnFinish (object sender, EventArgs args) => tcs.TrySetResult(true);
  void OnStop(){
    CleanupFoo(animation);
    tcs.TrySetResult(false);
  }

  // Null-safe cancellation 
  token.Register(OnStop);
  RunFoo(animation, OnFinish);
  return tcs.Task;
}

You can also return more complex results, eg a result type that contains a Finished/Stopped flag and the final frame if the animation was stopped. If you don't want to use meaningless fields (why specify a frame if the animation completed?), you can return a Success type or a Stopped type that implement eg IResult.

Before C# 7, you'd need to check the return type or use overloading to access different types. With pattern matching though, you can get the actual result with a switch, eg:

interface IResult{}
public class Success:IResult{}

public class Stopped { 
    public int Frame{get;}
    Stopped(int frame) { Frame=frame; }
}

....

var result=await Run(...);
switch (result)
{
    case Success _ : 
        Console.WriteLine("Finished");
        break;
    case Stopped s :
        Console.WriteLine($"Stopped at {s.Frame}");
        break;
}

Pattern matching is actually faster than type checking too. This requires that the client supports C# 7.

Answer 3

What you are doing is fine - Task represents some operation with result in future, it is not necessary related to running anything on another thread or something like that. And it's perfectly normal to use standard means of cancellation for, well, cancellation, instead of returning something like boolean value.

To answer your questions: when you do tcs.TrySetCanceled() it will move task to cancelled state (task.IsCancelled will be true) and no exceptions are thrown at this point. But when you await this task - it will notice that task is cancelled and that is the point where TaskCancelledException will be thrown. Nothing is wrapped into aggregate exception here because there is really nothing to wrap - TaskCancelledException is thrown as a part of await logic. Now if you will do something like task.Wait() instead - then it will wrap TaskCancelledException into AggregateException as you would expect.

Note that await unwraps AggregateExceptions anyway, so you might never expect await task to throw AggregateException - in case of multiple exceptions only first one will be thrown - the rest will be swallowed.

Now if you are using cancellation token with regular tasks - things are a bit different. When you do something like token.ThrowIfCancellationRequested it will actually throw OperationCancelledException (note that it's not TaskCancelledException but TaskCancelledException is subclass of OperationCancelledException anyway). Then, if CancellationToken used to throw this exception is the same as CancellationToken passed to task when it was started (like in example by your link) - task will move to the Cancelled state the same way. This is the same as tcs.TrySetCancelled in your code with the same behavior. If tokens mismatch - task will go to Faulted state instead, like the regular exception was thrown.