When reviewing work by a contractor, I came upon the following extension method:

public static class TaskExtensions
{
    public static async Task<IEnumerable<TResult>> WhenAllSynchronousExecution<TResult>(this IEnumerable<Task<TResult>> tasks, CancellationToken token = default(CancellationToken))
    {
        var results = new List<TResult>();
        var exceptions = new List<Exception>();
        foreach (var task in tasks)
        {
            if (task == null)
            {
                throw new ArgumentNullException(nameof(tasks));
            }
            
            if (token.IsCancellationRequested) {
                exceptions.Add(new OperationCanceledException("Tasks collection cancelled", token));
                break;
            }

            try
            {
                results.Add(await task);
            }
            catch (Exception ex)
            {
                exceptions.Add(ex);
            }
        }
        
        if (exceptions.Any()) {
            throw new AggregateException(exceptions);
        }
        
        return results;
    }
}

To this looks to be exactly what Task.WhenAll<TResult> already does. quote the remarks:

The call to WhenAll(IEnumerable<Task>) method does not block the calling thread. However, a call to the returned Result property does block the calling thread.

If any of the supplied tasks completes in a faulted state, the returned task will also complete in a Faulted state, where its exceptions will contain the aggregation of the set of unwrapped exceptions from each of the supplied tasks.

If none of the supplied tasks faulted but at least one of them was canceled, the returned task will end in the Canceled state.

If none of the tasks faulted and none of the tasks were canceled, the resulting task will end in the RanToCompletion state. The Task.Result property of the returned task will be set to an array containing all of the results of the supplied tasks in the same order as they were provided (e.g. if the input tasks array contained t1, t2, t3, the output task's Task.Result property will return an TResult[] where arr[0] == t1.Result, arr[1] == t2.Result, and arr[2] == t3.Result).

If the tasks argument contains no tasks, the returned task will immediately transition to a RanToCompletion state before it's returned to the caller. The returned TResult[] will be an array of 0 elements.

So to me it seems they've recreated an existing method. The only difference seems to be the checking of the CancellationToken, but I don't see mych added value there (it's not used in the code). And maybe that it's an extension method, but that's also not used in the code (it's called like var task = TaskExtensions.WhenAllSynchronousExecution(tasks);)

But this colleague is quite "sensitive". So I need to pick my battles. So is this just a custom,meagre recreation of Task.WhenAll<TResult>?

CodePudding user response：

Nope, there are quite a lot of differences between the Task.WhenAll and the WhenAllSynchronousExecution. Here are some of them:

1. The Task.WhenAll materializes immediately the IEnumerable<Task<TResult>> sequence, by enumerating it and storing the tasks in an array. This ensures that all the tasks are up and running, before attaching continuations on them. The WhenAllSynchronousExecution doesn't do that, so it could potentially start each task after the completion of the previous task (sequentially), depending on how the enumerable sequence is implemented.
2. The Task.WhenAll doesn't take a CancellationToken, so when it completes you can be 100% sure that all the tasks have completed. It is always possible to fire-and-forget a Task.WhenAll task, just like any other Task, with the WaitAsync method (.NET 6), so there is no reason to bake this functionality in every async method.
3. The Task.WhenAll propagates the exceptions directly, as the InnerExceptions of the Task.Exception property. The WhenAllSynchronousExecution wraps the exceptions in an additional AggregateException. So you can't switch between the two APIs without rethinking your exception-handling code.
4. The Task.WhenAll completes as Canceled when none of the tasks is Faulted, and at least one is Canceled. The WhenAllSynchronousExecution propagates the cancellation as error, always.
5. The WhenAllSynchronousExecution captures the SynchronizationContext in its internal await points, so it might cause a deadlock if the caller blocks on async code.
6. The Task.WhenAll checks for null task instances synchronously. The WhenAllSynchronousExecution does the check during the loop that awaits each task.

Correction: Actually the integrated CancellationToken is a useful feature, that has been requested on GitHub.

CodePudding user response：

I think the key to figuring out if this method has any reason to exist is to confirm if the tasks are started before the method is called.

1. If the tasks are not started then the method should probably be called ~WhenAllSequential.
2. If the tasks are started then this method seems not to add anything good.

In any case, I would argue against having this method as an extension method:

1. Having this method shown in auto-completion hints will sooner or later make someone pass already started tasks and think the tasks will be executed sequentially but it's the caller of this method who is in charge when the tasks are started.
2. Even the official WhenAll is not an extension method.