I have two long running tasks that I want to execute, to be followed with a continuation. The problem I am having is that the continuation is running before the two long running tasks have signalled. Those two tasks are not erroring, they've barely started when the continuation commences. The intent is that the first two tasks can run in parallel, but the continuation should only run once they've both finished.

I have probably overlooked something really simple, but near as I can tell I am following the patterns suggested in the question Running multiple async tasks and waiting for them all to complete. I realise I could probably dispense with the .ContinueWith() as the code following the Task.WhenAll() will implicitly become a continuation, but I prefer to keep the explicit continuation (unless it is the cause of my issue).

private async Task MyMethod()
{
    var tasks = new List<Task>
    {
        new Task(async () => await DoLongRunningTask1()),
        new Task(async () => await DoLongRunningTask2())
    };

    Parallel.ForEach(tasks, async task => task.Start());

    await Task.WhenAll(tasks)
    .ContinueWith(async t =>{
                                //another long running bit of code with some parallel
                                //execution based on the results of the first two tasks
                            }
                            , TaskContinuationOptions.OnlyOnRanToCompletion);

}

private async Task DoLongRunningTask1()
{
    ... long running operation ...
}

private async Task DoLongRunningTask2()
{
    ... another long running operation ...
}

I appreciate any help or suggestions!

CodePudding user response：

You are overusing async. Below is more cleaner rewrite of what you have done above.

private async Task MyMethod()
{
    var tasks = new Task[] {DoLongRunningTask1(), DoLongRunningTask2()];

    await Task.WhenAll(tasks);
    //another long running bit of code with some parallel
    //execution based on the results of the first two tasks

}

private async Task DoLongRunningTask1()
{
    ... long running operation ...
}

private async Task DoLongRunningTask2()
{
    ... another long running operation ...
}

I assumed DoLongRunningTask's are async in nature, if they are syncronious then start them using Task.Run within a ThreadPool thread and use resulting Task.

CodePudding user response：

Your code has multiple issues:

1. Creating tasks with the non-generic Task constructor, with async delegate. This constructor accepts an Action, not a Func<Task>, so your async delegate is actually an async void, which is something to avoid. You can fix this by creating nested Task<Task>s, where the outer task represents the launching of the inner task.
2. Using the Parallel.ForEach with async delegate. This again results in async void delegate, for the same reason. The correct API to use when you want to parallelize asynchronous delegates is the Parallel.ForEachAsync, available from .NET 6 and later.
3. Using parallelism to start tasks. Starting a task with Start is not CPU-bound, unless you are passing it (or being inside an ambient Current) exotic TaskScheduler, that schedules the execution of the task synchronously on the current thread. By default the execution is scheduled on the ThreadPool. The Start doesn't execute the task, it just schedules it for execution. So you can replace the Parallel.ForEach with a simple foreach loop or the List.ForEach.
4. Awaiting an unwrapped ContinueWith continuation that has an async delegate. This continuation returns a nested Task<Task>. You should Unwrap this nested task, otherwise you are awaiting the launching of the asynchronous operation, not its completion.

Here is your code "fixed":

private async Task MyMethod()
{
    List<Task<Task>> tasksOfTasks = new()
    {
        new Task<Task>(async () => await DoLongRunningTask1()),
        new Task<Task>(async () => await DoLongRunningTask2())
    };

    tasksOfTasks.ForEach(task => task.Start());

    await Task.WhenAll(tasksOfTasks.Select(taskTask => taskTask.Unwrap()))
    .ContinueWith(async t =>
    {
        //another long running bit of code with some parallel
        //execution based on the results of the first two tasks
    }, TaskContinuationOptions.OnlyOnRanToCompletion).Unwrap();
}

In case of failure in the DoLongRunningTask1 or the DoLongRunningTask2 methods, this code will most likely result in a TaskCanceledException, which is what happens when you pass the TaskContinuationOptions.OnlyOnRanToCompletion flag and the antecedent task does not ran to completion successfully. This behavior is unlikely to be what you want.

The above code also violates the guideline CA2008: Do not create tasks without passing a TaskScheduler. This guideline says that being depended on the ambient TaskScheduler.Current is not a good idea, and specifying explicitly the TaskScheduler.Default is recommended. You'll have to specify it in both the Start and ContinueWith methods.

If you think that all this is overly complicated, you are right. That's not the kind of code that you want to write and maintain, unless you have spend months studying the Task Parallel Library, you know it in and out, and you have no other higher-level alternative. It's the kind of code that you might find in specialized libraries, not in application code.