Search code examples
c#asynchronousconcurrency

Possibly redundant use of Task.FromResult


Section 10.2 within Concurrency in .NET provides the following listing for an Otherwise task combinator:

public static Task<T> Otherwise<T>(this Task<T> task, Func<Task<T>> orTask) =>
    task.ContinueWith(async innerTask =>
    {
        if (innerTask.Status == TaskStatus.Faulted) return await orTask();
        return await Task.FromResult<T>(innerTask.Result);
    }).Unwrap();

(Confusingly, within the printed book, Otherwise is marked as async which fails to compile; however, not in the GH listing which compiles without issue)

My "understanding" is that if the anonymous continuation is called, it would suggest that the supplied innerTask has completed by that point, successfully or otherwise.

Assuming successful, it would also suggest that innerTask.Result would already be determined with no synchronous waiting required upon calling that member property.

On the mild assumption that above is sensible... What possible benefit is there from using return await Task.FromResult<T>(innerTask.Result) as opposed to just return innerTask.Result? (the latter of which also compiles).

Unless I'm missing something (quite likely)... It feels overly verbose.


Update:

Thank you to everyone who has contributed; I never anticipated this would get as much feedback as it has!

At one point, I was concerned about a situation where innerTask.Result could raise an exception and how that would be dealt with by the 2x return approaches above.

From my own testing since, I can see that if innerTask was cancelled and, hence innerTask.Status == TaskStatus.Cancelled, the combinator implementation as given would permit the accessing of innerTask.Result which would then raise an AggregateException.

However, there is no difference in how this (or any other exception I imagine) would be treated by either return approach from within an async function.

Given this, and the information provided, it appears as though the former approach offers nothing over and above the latter. If anything, it (IMHO) obscurs the intention and has sent me down a (albeit illuminating) rabbit hole.


Solution

  • What possible benefit is there from using return await Task.FromResult<T>(innerTask.Result) as opposed to just return innerTask.Result?

    Absolutely zero benefit. That's the kind of code that your end up writing when you are struggling with a concept that you don't fully understand, and you reach to a version that compiles and works correctly. Your code might end up decorated with some useless cargo, which nonetheless is not harmful, so it might survive multiple code reviews and revisions.

    Here is a better way to write the Otherwise extension method:

    public static Task<T> Otherwise<T>(this Task<T> task, Func<Task<T>> onErrorTaskFactory)
    {
        return task.ContinueWith(innerTask =>
        {
            if (innerTask.IsFaulted) return onErrorTaskFactory();
            return innerTask;
        }, default, TaskContinuationOptions.DenyChildAttach |
            TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default).Unwrap();
    }
    

    Notice that the continuation is not async, and it returns the innerTask, not its result. With this version you get these benefits:

    1. In case the onErrorTaskFactory() returns a Task that also fails, and it contains more that one exceptions (think of passing a lambda that return Task.WhenAll()), all the errors will be propagated, not just the first one.
    2. In case the innerTask is canceled, the cancellation will be propagated without an additional internal try/catch. Throwing and catching exceptions in C# is quite expensive.
    3. The code complies with the CA2008 guideline ("Do not create tasks without passing a TaskScheduler").
    4. The onErrorTaskFactory will be invoked synchronously on the thread that completed the innerTask, avoiding a thread-switch.

    These benefits are not ground-breaking. I would evaluate the original implementation in the book as quite decent. It doesn't have any terrible bugs or inefficiencies.