2

Task.WhenAll(IEnumerable<Task>) waits for all tasks in the IEnumerable are complete --- but only the tasks in the list when it's first called. If any active task adds to the list, they aren't considered. This short example demonstrates:

    List<Task> _tasks = new List<Task>();

    public async Task  QuickExample()
    {
        for(int n =0; n < 6; ++n)
            _tasks.Add(Func1(n));

        await Task.WhenAll(_tasks);     
        Console.WriteLine("Some Tasks complete");

        await Task.WhenAll(_tasks);
        Console.WriteLine("All Tasks complete");
    }


    async Task Func1(int n)
    {
        Console.WriteLine($"Func1-{n} started");
        await Task.Delay(2000);
        if ((n % 3) == 1)
            _tasks.Add(Func2(n));
        Console.WriteLine($"Func1-{n} complete");
    }

    async Task Func2(int n)
    {
        Console.WriteLine($"Func2-{n} started");
        await Task.Delay(2000);
        Console.WriteLine($"Func2-{n} complete");
    }

This outputs:

Func1-0 started
Func1-1 started
Func1-2 started
Func1-3 started
Func1-4 started
Func1-5 started
Func1-5 complete
Func1-3 complete
Func2-1 started
Func1-1 complete
Func1-0 complete
Func1-2 complete
Func2-4 started
Func1-4 complete
Some Tasks complete
Func2-4 complete
Func2-1 complete
All Tasks complete
Done

The second Task.WhenAll() solves the problem in this case, but that's a rather fragile solution. What's the best way to handle this in the general case?

James Curran
  • 101,701
  • 37
  • 181
  • 258
  • Suppose a different thread adds tasks to `_tasks`. How should Task.WhenAll know adding is completed? – Eser May 20 '18 at 19:00
  • 1
    You can try to make the methods you're calling more "functional". The method can return the collection of tasks it spawned internally. That way you can keep your code clean and easy to reason about. Imho, other possible solutions are "hacky". – Sergey.quixoticaxis.Ivanov May 20 '18 at 19:03
  • You need to ```WaitAll``` only the list of known tasks. If you're wanting to add to it and continue waiting then I highly suggest a different approach. This is damaging in many ways, especially if you're not the only developer. If you have a thread adding tasks then wait that thread then ```WaitAll``` the rest when it's done adding them. Just a suggestion not a solution. – Michael Puckett II May 20 '18 at 19:06
  • Whoops, almost forgot. If you go with functional style and return spawned tasks, there would be less problems if parallelism would be introduced later on. – Sergey.quixoticaxis.Ivanov May 20 '18 at 19:09
  • 2
    You could simply `await` the new tasks from the tasks that generated them, without cascading them to the `_tasks` collection. If A creates B, then A doesn't finish until B finishes. – xanatos May 20 '18 at 19:19
  • How do you know if you're finished? All the tasks in the list are finished, right? But some more might be added... They might be added after the initial tasks have finished... Sounds really confusing with edge cases that I don't think you've properly considered. – spender May 20 '18 at 20:25
  • 2
    Have you considered using Microsoft's Reactive Framework for this? It's far more suited to this kind of thing. – Enigmativity May 20 '18 at 23:37

4 Answers4

2

You are modifying the List<> without locking it... You like to live a dangerous life :-) Save the Count of the _tasks before doing a WaitAll, then after the WaitAll check the Count of _tasks. If it is different, do another round (so you need a while around the WaitAll.

int count = _tasks.Count;

while (true)
{
    await Task.WhenAll(_tasks);

    lock (_tasks)
    {
        if (count == _tasks.Count)
        {
            Console.WriteLine("All Tasks complete");
            break;
        }

        count = _tasks.Count;
        Console.WriteLine("Some Tasks complete");
    }
}

async Task Func1(int n)
{
    Console.WriteLine($"Func1-{n} started");
    await Task.Delay(2000);

    if ((n % 3) == 1)
    {
        lock (_tasks)
        {
            _tasks.Add(Func2(n));
        }
    }

    Console.WriteLine($"Func1-{n} complete");
}

I'll add a second (probably more correct solution), that is different from what you are doing: you could simply await the new Tasks from the Tasks that generated them, without cascading them to the _tasks collection. If A creates B, then A doesn't finish until B finishes. Clearly you don't need to add the new Tasks to the _tasks collection.

xanatos
  • 109,618
  • 12
  • 197
  • 280
  • Mmm, why should the author synchronize access to the list? The author is not doing anything in parallel. – Sergey.quixoticaxis.Ivanov May 20 '18 at 19:01
  • 2
    @Sergey.quixoticaxis.Ivanov He is doing a `_tasks.Add(Func2(n));` inside the `async Task Func1(int n)`. There is no guarantee on the number of threads that will be used to run the `Task.WhenAll` with multiple `Task Func1()` to be executed. – xanatos May 20 '18 at 19:04
  • `async` is asynchronous, not parallel. – Sergey.quixoticaxis.Ivanov May 20 '18 at 19:05
  • 1
    @Sergey.quixoticaxis.Ivanov async is asynchronous, but it could become partially parallel, depending on how the task is written... See for example [here](https://stackoverflow.com/a/20622253/613130) *so it may run with as few as 2 threads or as many as 5* – xanatos May 20 '18 at 19:10
  • xanatos, sure, it could spawn a thousand threads inside, it has little to do with asynchronicity though. Btw Stephen Cleary had a great post about it if my memory serves me well. As I wrote in my comments to the original post, imho, the functional style would solve the OP's problem and live well both in single-threaded or multi-threaded code. – Sergey.quixoticaxis.Ivanov May 20 '18 at 19:14
  • @Sergey.quixoticaxis.Ivanov OP runs 6 Func1 tasks in parallel, so cantos is correct that it's not safe to add items to the List inside Func1. – Evk May 20 '18 at 20:34
  • @Evk no, he does not, and it can be easily checked by outputting managed thread identifier. The later parts of methods can run in parallel (only if the code runs without a synchronizing context). More precisely, the code before `await` runs on the same thread. The code after `await` can be run on the same thread or on different threads, depending on the context. Feel free to correct me if I miss something. – Sergey.quixoticaxis.Ivanov May 20 '18 at 20:57
  • Anyway, modifying a collection of tasks on the fly is a way to make code messy. The OP probably should go with @xanatos advise of using `await` on nested functions, or follow my advise of returning collections. – Sergey.quixoticaxis.Ivanov May 20 '18 at 21:01
1

Asynchronous function will return to the caller on first await.
So for loop will be complete before you add extra tasks to original tasks list.

Implementation of Task.WhenAll will iterate/copy tasks to local list, so added tasks after Task.WhenAll called will be ignored.

In your particular case moving call to Func1 before await Task.Delay() could be a solution.

async Task Func1(int n)
{
    Console.WriteLine($"Func1-{n} started");
    if ((n % 3) == 1)
        _tasks.Add(Func2(n));

    await Task.Delay(2000);
    Console.WriteLine($"Func1-{n} complete");
}

But if in real scenario calling of Func2 depend on result of some asynchronous method, then you need some other solution.

Fabio
  • 31,528
  • 4
  • 33
  • 72
1

Since it seems that additional tasks can be created during the course of executing the original list of tasks, you will need a simple while construct.

while (_tasks.Any( t => !t.IsCompleted ) 
{
    await Task.WhenAll(_tasks);
}

This will check the list for any uncompleted tasks and await them until it catches the list at a moment when there are no tasks left.

John Wu
  • 50,556
  • 8
  • 44
  • 80
  • I think this solution is more preferable than the top voted answer. This works even if Tasks get removed from the list for example the Count could stay the same. Although this is much worse on performance. One thing that could be done is traverse the List backwards because it is more likely that one of the last tasks is not completed. Also instead of a lock i would prefer a Semaphore. – Stan Jul 29 '23 at 12:51
0

Consider this; it sounds like work is being submitted to the "Task List" from another thread. In a sense, the "task submission" thread itself could also be yet another Task for you to wait on.

If you wait for all Tasks to be submitted, then you are guaranteed that your next call to WhenAll will yield a fully-completed payload.

Your waiting function could/should be a two-step process:

  1. Wait for the "Task Submitting" task to complete, signalling all Tasks are submitted
  2. Wait for all the submitted tasks to complete.

Example:

public async Task WaitForAllSubmittedTasks()
{
    // Work is being submitted in a background thread;
    // Wrap that thread in a Task, and wait for it to complete.
    var workScheduler = GetWorkScheduler();
    await workScheduler;

    // All tasks submitted!

    // Now we get the completed list of all submitted tasks.
    // It's important to note: the submitted tasks
    // have been chugging along all this time.
    // By the time we get here, there are probably a number of
    // completed tasks already.  It does not delay the speed
    // or execution of our work items if we grab the List
    // after some of the work has been completed.
    //
    // It's entirely possible that - by the time we call
    // this function and wait on it - almost all the 
    // tasks have already been completed!
    var submittedWork = GetAllSubmittedTasks();
    await Task.WhenAll(submittedWork);

    // Work complete!
}
BTownTKD
  • 7,911
  • 2
  • 31
  • 47