CODEWITHSUNDEEP
c#async-awaittaskcompletionsource
J4ni
J4ni

Reputation: 217

Return with await when wrapping old async pattern into TaskCompletionSource?

I am studying C# Asnc-await pattern and currently reading Concurrency in C# Cookbook from S. Cleary

He discusses wrapping old non TAP async patterns with TaskCompletionSource (TCS) into TAP constructs. What I dont get is, why he just returns the Task property of the TCS object instead of awaiting it TCS.Task ?

Here is the example code:

Old method to wrap is DownloadString(...):

public interface IMyAsyncHttpService
{
    void DownloadString(Uri address, Action<string, Exception> callback);
}

Wrapping it into TAP construct:

public static Task<string> DownloadStringAsync(
    this IMyAsyncHttpService httpService, Uri address)
{
    var tcs = new TaskCompletionSource<string>();
    httpService.DownloadString(address, (result, exception) =>
    {
        if (exception != null)
            tcs.TrySetException(exception);
        else
            tcs.TrySetResult(result);
    });
    return tcs.Task;
}

Now why not just do it that way:

public static async Task<string> DownloadStringAsync(
    this IMyAsyncHttpService httpService, Uri address)
{
    var tcs = new TaskCompletionSource<string>();
    httpService.DownloadString(address, (result, exception) =>
    {
        if (exception != null)
            tcs.TrySetException(exception);
        else
            tcs.TrySetResult(result);
    });
    return await tcs.Task;
}

Is there a functional difference between the two? Is the second one not more natural?

Upvotes: 1

Views: 534

Answers (5)

haimb
haimb

Reputation: 419

Read my answer why returning the task is not a good idea: What is the purpose of "return await" in C#?

Basically you break your call stack if you don't use await.

Upvotes: 0

J4ni
J4ni

Reputation: 217

Folks, thanks for the helpful comments.

In the meantime I have read the sources referenced here and also investigated the matter further: Influenced by https://blog.stephencleary.com/2016/12/eliding-async-await.html I have come to the conclusion, that its best practice to include async-await by default even in synchronous methods of the async function chain and only omit async await when the circumstances clearly indicate that the method will not potentially behave differently as expected from an async method en edge scenarios.
Such as: the synchronous method is short, simple and has no operations inside which could throw an exception. If the synchronous method throws an exception the caller will get an exception in the calling line instead of the line where the Task is awaited. This is clearly a change from expected behavior.

For example handing over call parameters to the next layer unchanged is a situation which imo permits omitting async-await.

Upvotes: 0

Damien_The_Unbeliever
Damien_The_Unbeliever

Reputation: 239814

By marking it async, the compiler will generate warnings, that it should be considered to await this method

You don't have to mark your own method as async in order to get the "Task not awaited" warning. The following code generates the same warning for the calls to both T and U:

static async Task Main(string[] args)
{
    Console.WriteLine("Done");
    T();
    U();
    Console.WriteLine("Hello");
}

public static Task T()
{
    return Task.CompletedTask;
}

public static async Task U()
{
    await Task.Yield();
    return;
}

Whenever you find yourself with a method only containing a single await and that being the last thing it does (except possibly returning the awaited value), you should ask yourself what value it's adding. Aside from some differences in exception handing, it's just adding an extra Task into the mix.

await is generally a way of indicating "I've got no useful work to do right now, but will have when this other Task is finished" which of course isn't true (you've got no other work to do later). So skip the await and just return what you would have awaited instead.

Upvotes: 2

canton7
canton7

Reputation: 42330

There is one subtle practical difference (other than the version with await being slower to run).

In the first example, if DownloadString throws an exception (rather than calling the delegate you pass it with exception set), then that exception will bubble through your call to DownloadStringAsync.

In the second, the exception is packaged into the Task returned from DownloadStringAsync.

So, assuming that DownloadString throws this exception (and no other exceptions occur):

Task<string> task;
try
{
    task = httpService.DownloadStringAsync(...);
}
catch (Exception e)
{
    // Catches the exception ONLY in your first non-async example
}

try 
{
    await task;
}
catch (Exception e)
{
    // Catches the exception ONLY in your second async example
}

You probably don't care about the distinction - if you just write:

await httpService.DownloadStringAsync(...);

you won't notice the difference.

Again, this only happens if the DownloadString method itself throws. If it instead calls the delegate you give it with exception set to a value, then there is no observable difference between your two cases.

Upvotes: 0

Martijn
Martijn

Reputation: 12102

Your version is strictly more complex -- instead of just returning the task, you make the method async, and await the task you could just be returning.

Upvotes: 0

Related Questions