Adding an anonymous Task to List<Task> does not execute it after calling .WaitAll() C#

Question

I add a task to a task list the following way

taskList.Add(new Task(async () =>
    {
        // work here
        await MethodInsideThatNeedsAwaiting(); // if that has anything to do with it
        // more work 
    }));

Calling Task.WaitAll(tasklist); afterwards gets "stuck". The program continues to run but there's nothing heard from any tasks from that list anymore nor does it hit any breakpoints as if it's stuck in its own internal async loop somewhere.

Is the way I'm adding the task to the list wrong? What's the issue here?

What I also tried:
I've tried the following as well, if for some reason the async keyword was the problem, but it still doesn't work:

taskList.Add(new Task(() =>
    {
        // work here
        MethodInsideThatNeedsAwaiting().Wait();
        // more work 
    }));

This however works as expected

private async Task GetStuff()
{
    // same work here
    await MethodInsideThatNeedsAwaiting();
    // more work
}

And then adding it with taskList.Add(GetStuff()); Calling Task.WaitAll(tasklist); has no problems with this one.

Answer 1

Everything that you're doing here is wrong. Stop what you're doing and learn how this works before you write more asynchronous code. You are setting yourself up for tasks which never complete because they never started, and for tasks which never complete because they are waiting on themselves. Asynchronous workflows are a little tricky.

First off: you almost never want to use new Task. It just means "make a task that represents this work". It does not mean to execute that work. new Task makes a to-do list; it does not do the stuff on the list!

Second, you almost never want to use Task.Run. That means to make a task that represents the work and assign a worker from the pool to run it. You don't want to allocate a worker unless the work that you're doing is synchronous and CPU bound, which your work is not.

Third, you almost never want to use either on something that is already a task. You have an async lambda in hand. It returns a task when invoked, so if you want a task for a running workflow, invoke the lambda!

Fourth, you almost never want to WaitAll. That destroys the whole point of asynchrony by turning an asynchronous workflow back into a synchronous workflow.

Fifth, you almost never want to call Wait on a task, for the same reason. But it gets worse! Here's a to-do list I'd like you to do: First, put the bread in the toaster and start it toasting. Second, synchronously wait for the sandwich to be done; do not move on until this step is complete. Third, eat the sandwich. Fourth, when the toaster pops, take the toast out of the toaster, and put some ham on the toast to make a sandwich. You will wait forever if you try to execute this workflow. Asynchronous workflows deadlock when you insert synchronous waits into them because you are often in a situation where you are waiting on work that you yourself are going to do in the future.

(An aside about that last point: if you are in that situation, the correct solution is NOT to change the second step of the workflow to "hire a worker to complete my sandwich, and synchronously wait for that worker to be done". You often see this sort of bizarre, wasteful fix to an incorrect workflow. When you remove synchronous waits and insert asynchronous waits (await) at points where the workflow cannot continue until a task is complete then you will find that your workflows can be done all one one thread.)

Everything you are doing is absolutely the wrong way to do asynchronous programming, and you will not be successful if you keep on doing it like this.

OK, so now that you know how not to do it, how do you do it?

• If you have a method or lambda that returns a task, invoke it to get the task.
• If you need to wait for that task to complete before your workflow can continue, await the task.
• If you have several tasks and you need them all to complete before your workflow can continue, put them in a collection and then call WhenAll to get a new task back that represents the task of completing all those tasks. Then you must await that task.

Some correct workflows:

Here's the simplest one:

private async Task GetStuffAsync()
{
    // same work here
    await MethodInsideThatNeedsAwaitingAsync();
    // more work
}
private async Task DoItAsync()
{
    // do work
    await GetStuffAsync();
    // do more work
}

What if you have multiple tasks and you want to wait for all of them, but they don't have to wait for each other?

private async Task DoItAsync()
{
    // do work
    Task t1 = GetStuffAsync();
    // do work
    Task t2 = GetOtherStuffAsync();
    // do more work
    // We cannot continue until both are done
    await t1;
    await t2;
    // do even more work
}

What if you have an unknown number of such tasks?

private async Task DoItAsync()
{
    // do work
    var tasks = new List<Task>();
    while (whatever)
      tasks.Add(GetStuffAsync()); 
    // do work
    // we cannot continue until all tasks are done
    await Task.WhenAll(tasks);
    // do more work
}