Task.Delay not delaying async method

Question

I'm trying to create multiple tasks, run them in parallel, and wait for them all to finish.

public class SimulationManager
{
    public List<Task> Simulations = new List<Task>();

    public void AddSimulation(SimulationParameters parameters)
    {
        Simulations.Add(new Task(async () => await new Simulation().Simulate()));
    }

    public async Task StartSimulations()
    {
        Simulations.ForEach(s => s.Start());
        await Task.WhenAll(Simulations);

        Console.WriteLine("All tasks finished");
    }
}

The task itself delays the execution by one second and then prints out a message.

public class Simulation
{
    public async Task Simulate()
    {
        Console.WriteLine("Simulating");
        await Task.Delay(1000);
    }
}

I would expect the output to be:

Simulating
All tasks finished

Instead, I get:

All tasks finished
Simulating

If I replace await Task.Delay(1000) with Thread.Sleep(1000) it works as expected.

Why is the task being marked as completed without actually being completed?

If I read the status of the task before and after Task.WhenAll, it is awaiting correctly. The problem is then that Task.Delay is not delaying the execution even though the method is async.

Simulations.ForEach(s => s.Start());
Console.WriteLine(Simulations.First().Status); // prints "WaitingToRun"
await Task.WhenAll(Simulations);
Console.WriteLine(Simulations.First().Status); // prints "RanToCompletion"
Console.WriteLine("All tasks finished");

You are misusing tasks in multiple ways: 1. Tasks are not for parallel execution. That’s what the Parallel class is for. Tasks are primarily for asynchronous execution. 2. You should use Task.Run instead of manually starting them. 3. There isn’t really a point to wrap a Task.Delay in another Task. W.r.t. your question: the issue is that you are using the task constructor (which should usually not be used) and thus one only accepts delegates of type Action. So it is not aware that there is a task inside and thus doesn’t await it. So the inner task just gets kicked of and the delegate exits. — ckuri, Oct 31 '20 at 23:25
You are probably looking for something like this `Simulations.Add(new Simulation().Simulate());` and don't do this `Simulations.ForEach(s => s.Start())` those tasks will already be started *hot* — TheGeneral, Oct 31 '20 at 23:28
If you would start the task properly by using Task.Run, which accept a Func delegate and thus is aware of inner tasks, you would get the proper result. — ckuri, Oct 31 '20 at 23:30
Also there is no need to *offload* a task using `Task.Run` at all — TheGeneral, Oct 31 '20 at 23:32

Alois Kraus · Answer 1 · 2020-10-31T23:59:08.920

You are waiting on the wrong thing. Here is a fixed version

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace TaskSimulateEarlyReturn
{
    public class Simulation
    {
        public async Task Simulate()
        {
            Console.WriteLine("Simulating");
            await Task.Delay(1000);
            Console.WriteLine("Finished Simulating");
        }
    }

    public class SimulationManager
    {
        public List<Task<Task>> Simulations = new List<Task<Task>>();

        public void AddSimulation()
        {
            Simulations.Add(new Task<Task>(async () => await new Simulation().Simulate()));
        }

        public async Task StartSimulations()
        {
            for(int i=0;i<4;i++)
            {
                AddSimulation();
            }
            Simulations.ForEach(s => s.Start());
            await Task.WhenAll(Simulations.Select(x=>x.Unwrap()).ToArray());

            Console.WriteLine("All tasks finished");
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            var man = new SimulationManager();
            man.StartSimulations().Wait();
            Thread.Sleep(1000);
        }
    }
}

The key ingredient is that you are creating a task with an async method inside it. Implicitely an async method will always return a task which is complete when the async method has completed. Since you are wrapping the Task with a Task you are waiting on the outer unrelated task which completes immediately hence your race condition.

    async void F1()
    {
        await Task.CompletedTask;
    }

    async Task F2()
    {
        await Task.CompletedTask;
    }

Both async methods are identical but the F1 method which does return void will still return a task under the hood or else you would not be able to await it. This is just a compiler trick to make "old" code work with async methods, without packing async as special method signature on top of it.

You are creating then tasks like this:

    var t  = new Task(F1);
    var t1 = new Task<Task>(F2);

But now you have wrapped the async task returning method inside an outer task which will be complete as soon as the first sync part of the async method has completed. What you need to do is to wait on the inner task which can be conveniently done with Task.Unwrap which is there exactly for that reason.

If you remove the Task.Unwrap call in my sample you are waiting on the outer task and you are getting your race condition back.

In general I would not use async/await except to free up the UI thread. async/await is inherently single threaded because you can await always only one task. If used this way (Task.WhenAll is kind of cheating) the one and only feature you get is thread hopping. At a single point in time your async await things will run on one thread which might change before during and after the await depending on the Synchronization Contexts.

Thank you for your elaborate answer. I'm sure this would work but I like Fabio's approach above better due to its simplicity. That way I can also avoid having to call the Start() method on the task. — Johannes Mols, Oct 31 '20 at 23:56
As I said I would not use async/await in the first place unless I have to. The code looks simple but it is riddled with subtle race conditions if you are not very careful. Besides that async/await makes you not faster — Alois Kraus, Nov 01 '20 at 00:01
Upvoted. Nested tasks `Task` can be sometimes useful as simplified substitutes of a `TaskCompletionSource`. [Here](https://stackoverflow.com/questions/21424084/task-sequencing-and-re-entracy/62882637#62882637 "Task sequencing and re-entracy") is an example. — Theodor Zoulias, Nov 01 '20 at 00:15

Fabio · Accepted Answer · 2020-10-31T23:35:09.123

2

Remove redundant Task wrapper - this is what causing the issues.
Store simulations as a function returning Task and start simulation explicitly by invoking it.

public class SimulationManager
{
    public List<Func<Task>> Simulations = new List<Func<Task>>();

    public void AddSimulation(SimulationParameters parameters)
    {
        Simulations.Add(() => new Simulation().Simulate());
    }

    public async Task StartSimulations()
    {
        var tasks = Simulations.Select(simulate => simulate()).ToArray();
        await Task.WhenAll(tasks);

        Console.WriteLine("All tasks finished");
    }
}

edited Oct 31 '20 at 23:35

answered Oct 31 '20 at 23:33

Fabio

31,528
4
33
72

Like that... I personally would just `simulate()` - not really sure why so many people prefer `.Invoke()` - but that is purely style choice. – Alexei Levenkov Oct 31 '20 at 23:34
That's clean, I like it. I can see how I misused it in my attempt. – Johannes Mols Oct 31 '20 at 23:49

Task.Delay not delaying async method

2 Answers2