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:
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?TaskCanceledException being thrown unwrapped is also expected and normal and I'm not doing anything wrong here?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:
ThrowIfCancellationRequested. This option is better for CPU-bound code.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:
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.