2

I have the following class which is named Pluck:

internal static void Work()
{
    Task[] tasks = new Task[5];
    for (int i = 0; i < tasks.Length; i++)
    {
        tasks[i] = SumAsync();
    }
    Task.WhenAll(tasks);
}

private static async Task<int> SumAsync()
{
    return await Task.Run(() => { return OnePlusOne(); });
}

private static int OnePlusOne()
{  return 1+1;  }

And my main method:

static void Main(string[] args)
{
    Pluck.Work(); 
}

I am missing something because I toggle a breakpoint within OnePlusOne and never gets hit.

i3arnon
  • 113,022
  • 33
  • 324
  • 344
0x777
  • 825
  • 8
  • 14
  • 1
    `Task.WhenAll` is not a blocking method. It returns a Task that you can await. Use `Task.WaitAll` instead. – Mike Zboray Jan 08 '15 at 17:07
  • 1
    Also there is no point in using `async/await` in `SumAsync`. Just return the task created by `Task.Run`. – Panagiotis Kanavos Jan 08 '15 at 17:13
  • 2
    You must have something else wrong. A breakpoint on 1+1 stops on my machine. I did reformat the code to have curly braces on separate lines (as God intended). – Steve Wellens Jan 08 '15 at 17:18
  • @SteveWellens it's a race condition since the app can end before these tasks are scheduled. – i3arnon Jan 08 '15 at 17:21
  • @I3arnon - I don't think so. The operating system will not abruptly stop tasks with no error raised. If it is running under a debugger and there is a breakpoint....the break point will be hit. – Steve Wellens Jan 08 '15 at 18:09
  • @mikez WaitAll does the work. So WhenAll simply starts the threads but they won't do anything since the main thread ends when Work()? – 0x777 Jan 08 '15 at 18:30
  • 1
    @SteveWellens: tasks are run on background threads, which do not prevent the process from exiting. The OS certainly _will_ stop those threads with no error raised; they are simply terminated without ceremony if all foreground threads in the process exit. And that's exactly what happens here. – Peter Duniho Jan 08 '15 at 18:31
  • @defmx: neither `WaitAll()` nor `WhenAll()` "do work" per se. They simply wait for something else to complete. `WaitAll()` blocks; it won't return until all the tasks passed to it complete. `WhenAll()` is asynchronous. That is, it does _not_ block and instead returns a `Task` which itself can be waited on (e.g. using `await`). – Peter Duniho Jan 08 '15 at 18:35
  • @PeterDuniho - You are correct. I added some ` Console.Beep(440, 2000);` statements to the code to verify what you stated. Thanks! – Steve Wellens Jan 08 '15 at 23:01

1 Answers1

3

Task.WhenAll(tasks) is an async method and so returns a Task. You need to await that task to make sure you proceed only after all the tasks completed. Currently your application may end before these tasks get a chance to run.

This results in marking Work with the async keyword and have it return a Task itself:

internal static async Task WorkAsync()
{
    Task[] tasks = new Task[5];
    for (int i = 0; i < tasks.Length; i++)
    {
        tasks[i] = SumAsync();
    }
    await Task.WhenAll(tasks);
}

Since you can't use await in Main you need to synchronously wait with Wait:

static void Main(string[] args)
{
    Pluck.WorkAsync().Wait(); 
}

In cases where a single await is the last things you do in a method (as in your SumAsync and my WorkAsync) you can remove it and the async keyword and simply return the task instead. This slightly improves performance. More about it here.


Note: You should only block on a task with Wait in very specific cases (like inside Main) since it can lead to deadlocks. A better solution would be to use an AsyncContext.

Community
  • 1
  • 1
i3arnon
  • 113,022
  • 33
  • 324
  • 344
  • 1
    FWIW: the `WorkAsync()` method doesn't need to be `async` either. It also can just return the return value from the `WhenAll()` method. – Peter Duniho Jan 08 '15 at 18:35
  • @PeterDuniho of course. But it's a premature optimization with an insignificant added value. – i3arnon Jan 08 '15 at 19:00
  • Huh? It is _exactly_ the same "premature optimization" as your comment that "there's no use in making `SumAsync` `async`". Does that mean that your suggestion to not make `SumAsync()` an `async` method also has "insignificant added value"? If so, why did you bother to mention that? – Peter Duniho Jan 08 '15 at 19:44
  • 1
    lol...really? I think your answer was _much_ better when you pointed out the method didn't need to be `async`. I actually disagree that it's a "premature optimization" so much as it's a "don't write code that is pointless". Oh well...suit yourself. – Peter Duniho Jan 08 '15 at 19:48
  • 1
    Agreed with peter, it's not that the code is going to be slower (it is, but not enough to matter in most situations) but rather than its just adding unnecessary complexity to the code. Not adding superfluous code, particular code with as complex of implications as `async` has, is where the value comes from. When that complexity is actually needed to accomplish something, that's when it's worth using. – Servy Jan 08 '15 at 19:50
  • @Servy @PeterDuniho The added complexity comes when people need to think about whether they can remove the `async` but keep the task or not. When you're introducing these concepts to people they would sink in much better without that constant clarification. We, and anyone else who's interested, already knows about that. – i3arnon Jan 08 '15 at 19:53
  • @PeterDuniho I could also add that there's no use in creating a new lambda expression in each call and that *slightly* hurts performance. But that's really not the issue the OP cares about. – i3arnon Jan 08 '15 at 19:54
  • @PeterDuniho I could also use `Task.WaitAll` instead (which does the same in this case) and solve that `async` issue altogether... but that would send the wrong message. – i3arnon Jan 08 '15 at 19:56
  • The other things you mention are not simply about superfluous code. The `await` keyword has a specific meaning: "execute the operand asynchronously and continue execution after this statement when it completes". As the last statement of a method, it's pointless. Yes, the lambda could be omitted, but it adds expressiveness to the code. And of course use `WaitAll()` negates the whole point of the question. Those examples change the example significantly, while removing the `async` does not (and demonstrates good programming too). All IMHO of course. Your answer, your prerogative. :) – Peter Duniho Jan 08 '15 at 20:01