21

Usually I don't post a question with the answer, but this time I'd like to attract some attention to what I think might be an obscure yet common issue. It was triggered by this question, since then I reviewed my own old code and found some of it was affected by this, too.

The code below starts and awaits two tasks, task1 and task2, which are almost identical. task1 is only different from task2 in that it runs a never-ending loop. IMO, both cases are quite typical for some real-life scenarios performing CPU-bound work.

using System;
using System.Threading;
using System.Threading.Tasks;

namespace ConsoleApplication
{
    public class Program
    {
        static async Task TestAsync()
        {
            var ct = new CancellationTokenSource(millisecondsDelay: 1000);
            var token = ct.Token;

            // start task1
            var task1 = Task.Run(() =>
            {
                for (var i = 0; ; i++)
                {
                    Thread.Sleep(i); // simulate work item #i
                    token.ThrowIfCancellationRequested();
                }
            });

            // start task2
            var task2 = Task.Run(() =>
            {
                for (var i = 0; i < 1000; i++)
                {
                    Thread.Sleep(i); // simulate work item #i
                    token.ThrowIfCancellationRequested();
                }
            });  

            // await task1
            try
            {
                await task1;
            }
            catch (Exception ex)
            {
                Console.WriteLine(new { task = "task1", ex.Message, task1.Status });
            }

            // await task2
            try
            {
                await task2;
            }
            catch (Exception ex)
            {
                Console.WriteLine(new { task = "task2", ex.Message, task2.Status });
            }
        }

        public static void Main(string[] args)
        {
            TestAsync().Wait();
            Console.WriteLine("Enter to exit...");
            Console.ReadLine();
        }
    }
}

The fiddle is here. The output:

{ task = task1, Message = The operation was canceled., Status = Canceled }
{ task = task2, Message = The operation was canceled., Status = Faulted }

Why the status of task1 is Cancelled, but the status of task2 is Faulted? Note, in both cases I do not pass token as the 2nd parameter to Task.Run.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
noseratio
  • 59,932
  • 34
  • 208
  • 486

1 Answers1

12

There are two problems here. First, it's always a good idea to pass CancellationToken to the Task.Run API, besides making it available to the task's lambda. Doing so associates the token with the task and is vital for the correct propagation of the cancellation triggered by token.ThrowIfCancellationRequested.

This however doesn't explain why the cancellation status for task1 still gets propagated correctly (task1.Status == TaskStatus.Canceled), while it doesn't for task2 (task2.Status == TaskStatus.Faulted).

Now, this might be one of those very rare cases where the clever C# type inference logic can play against the developer's will. It's discussed in great details here and here. To sum up, in case with task1, the following override of Task.Run is inferred by compiler:

public static Task Run(Func<Task> function)

rather than:

public static Task Run(Action action)

That's because the task1 lambda has no natural code path out of the for loop, so it may as well be a Func<Task> lambda, despite it is not async and it doesn't return anything. This is the option that compiler favors more than Action. Then, the use of such override of Task.Run is equivalent to this:

var task1 = Task.Factory.StartNew(new Func<Task>(() =>
{
    for (var i = 0; ; i++)
    {
        Thread.Sleep(i); // simulate work item #i
        token.ThrowIfCancellationRequested();
    }
})).Unwrap();

A nested task of type Task<Task> is returned by Task.Factory.StartNew, which gets unwrapped to Task by Unwrap(). Task.Run is smart enough to do such unwrapping automatically for when it accepts Func<Task>. The unwrapped promise-style task correctly propagates the cancellation status from its inner task, thrown as an OperationCanceledException exception by the Func<Task> lambda. This doesn't happen for task2, which accepts an Action lambda and doesn't create any inner tasks. The cancellation doesn't get propagated for task2, because token has not been associated with task2 via Task.Run.

In the end, this may be a desired behavior for task1 (certainly not for task2), but we don't want to create nested tasks behind the scene in either case. Moreover, this behavior for task1 may easily get broken by introducing a conditional break out of the for loop.

The correct code for task1 should be this:

var task1 = Task.Run(new Action(() =>
{
    for (var i = 0; ; i++)
    {
        Thread.Sleep(i); // simulate work item #i
        token.ThrowIfCancellationRequested();
    }
}), token);
Community
  • 1
  • 1
noseratio
  • 59,932
  • 34
  • 208
  • 486
  • So when a token is implicitly passed `Task.Run` instead of explicitly via a parameter, it is considered as every other exception that happened inside the lambda? I tried browsing through the CancellationToken registration but couldn't figure much out of it in the source – Yuval Itzchakov Jun 23 '14 at 08:23
  • @YuvalItzchakov, that's true for `Task.Run`. However, for a plain `async Task` method the magic is done by the `async/await` compiler infrastructure code: `async Task TestAsync(CancellationToken token) { token.ThrowIfCancellationRequested(); }` would return a `Task` with `task.IsCanceled == true`, if cancellation was requested on the token. – noseratio Jun 23 '14 at 08:27
  • That's interesting. I wonder why they behave differently. Would an async lambda behave like `task1` or `task2`? or will it differ depending on the return delegate type? – Yuval Itzchakov Jun 23 '14 at 08:30
  • @YuvalItzchakov, me too. `Task.Run`, `Task.Factory.StartNew()`, `new Task()` and `Task.ContinueWith` give us an option to explicitly associate the token with the task they create. OTOH, an `async Task` method doesn't have that option, another C# syntax extension would be required for that. I guess, this might be the reason. – noseratio Jun 23 '14 at 08:37
  • @YuvalItzchakov, an `async` lambda would behave like `task1`, as it would create a nested task which `Task.Run` would unwrap. This however would be an expected behavior we could rely on. The trick with the `task1` from above is that the lambda is not `async`, but is still treated as `Func`. – noseratio Jun 23 '14 at 08:42
  • It seems like they are taking token by ref instead of by value. Throwing twice might be the problem – zahir Jun 23 '14 at 10:41
  • @zahir, not sure what you mean. A token, although of a value type, itself holds a reference to its parent `CancellationTokenSource` (same as `CancellationTokenRegistration` does). You can have multiple tokens referring to the same source, and cancellation is requested via the source, not via the token. – noseratio Jun 23 '14 at 10:50
  • @Noseratio I can't truely understand the compilers behavior regarding lambda overload resolution, as per the previous question by l3arnon. Cant really get my head around why the compiler is infering a `Func` from infinite loops. – Yuval Itzchakov Jun 23 '14 at 11:15
  • @YuvalItzchakov, check [this](http://stackoverflow.com/q/11941779/1768303) for hopefully a better explanation of it. – noseratio Jun 23 '14 at 11:17
  • @Noseratio I did read them both. What i dont understand is why a lambda with a return type other than `void` is considered a better resolution than a non `void` lambda. – Yuval Itzchakov Jun 23 '14 at 11:19
  • @YuvalItzchakov, I see. That's because `Task SomeMethod() { throw new Exception(); }` is a legal, compile-able method in C#. It doesn't return anything yet it can be cast to `Func`, and so can the lambda from `task1`. This lambda can also be cast to `Action`, but following up on @svick's [answer](http://stackoverflow.com/a/11941780/1768303) (the part in bold), the C# language specs dictate that `Func` is a more favorable choice, and so it goes with `task1`. – noseratio Jun 23 '14 at 11:27
  • 1
    Right. I'll think ill have to accept that fact until i can understand why MSFT made the choice of the "better overload" being the `Func` – Yuval Itzchakov Jun 23 '14 at 11:37
  • @YuvalItzchakov, I tend to agree, I'd also prefer to not have such implicit ambiguity in C#. Alternatively, the TPL designers might consider offering something like `Task.RunAsync(Func func, ...)` to kill a possibility for this ambiguity, rather than having those two overrides for `Task.Run()`. – noseratio Jun 23 '14 at 11:42
  • 2
    @Noseratio I mailed Eric Lippert and asked him to comment on the lambda inference. He answered: http://stackoverflow.com/a/24316474/1870803 – Yuval Itzchakov Jun 23 '14 at 13:53