Search code examples
c#asynchronoustasktask-parallel-librarycontinuations

Continuation is running before antecedents have finished


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!


Solution

  • 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. The Start doesn't execute the task, it just schedules it for execution, normally on the ThreadPool. 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(taskTask => taskTask.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 run 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 spent 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.