Is there anything wrong with this shortcut for a "happy path" for Task / ContinueWith?-CodePudding

For some heavy caching / high performance path in my project I need to "cast" a Task<X> to a Task<Y> type - see here.

I used the ContinueWith solution, but I noticed that even when my original task is in a RanToCompletion state, the ContinueWith still returns a Task that is WaitingToRun.

var task = Task.FromResult("cheese");
var test = task.ContinueWith(x => (object)x);
Console.WriteLine(test.Status); // WaitingToRun
Console.ReadLine();

For reasons out of the scope of this question, this gives me some extra cost further down the line.

If I do the following, my very specific problem seems to be fixed and it will directly return a CompletedTask, otherwise it does the normal logic.

if ( task.IsCompletedSuccessfully )
    return Task.FromResult((object)task.Result);
else
    return task.ContinueWith(x => (object)x);

Is it safe to add this "happy path" to my logic?

Because this path is not in the normal ContinueWith logic I am afraid I am missing something and this will bite me when running in production in a high concurrent environment.

CodePudding user response：

As is the case just about anytime ContinueWith comes up, the answer is to use await instead. Using ContinueWith correctly is really hard, and it's almost never the most effective solution to any given problem.

You've found one problem with ContinueWith, namely that without being really careful with your scheduler (or manually checking if the task is completed) you've ended up forcing work to be scheduled into a thread pool thread to run later when it just doesn't need to be. (That won't always happen, so you can't rely on that behavior, but it's possible, which is frankly, worse.)

Additionally, your ContinueWith code doesn't handle errors/cancellation properly. In your code cancelled tasks become errored instead, and errored tasks have their exception wrapped in an AggregateException.

Your code also has a bug in that you don't get the result from the task before casting it, and thus the result of your task is another task cast to an object, but that part is easily fixed by just adding in a Result call.

Here is a pre-await solution attempting to solve these problems, but in a post-await world, there's just no need.

public static async Task<TResult> Cast<TSource, TResult>(
    this Task<TSource> task)
{
    return (TResult)(object) await task.ConfigureAwait(false);
}

Not only is await a more effective syntax for scheduling continuations, but it's default behavior is much more likely to be correct in any given situation than the default behaviors of ContinueWith. It'll check if the task is completed and only schedule a continuation to run later if it's not, it doesn't wrap exceptions in an AggregateException, etc. The only default behavior that we don't want here is that by default await maintains the current synchronization context, which is often correct, but happens to not be needed for our method here.