Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Synchronous implementation of interface that returns Task

Similar to Implementing an interface that requires a Task return type in synchronous code although I'm curious if I should just ignore the compiler error my situation generates instead.

Let's say I have an interface like this:

public interface IAmAwesome {
    Task MakeAwesomeAsync();
}

In some implementations making awesome benefits from being done asynchronously using async and await. This is really what the interface is attempting to allow for.

In other cases, perhaps rare, only a simple synchronous method is needed to make awesome. So let's suppose that implementation looks like this:

public class SimplyAwesome : IAmAwesome {
    public async Task MakeAwesomeAsync() {
        // Some really awesome stuff here...
    }
}

This works, but the compiler is warning:

This method lacks 'await' operators and will run synchronously. Consider using the await operator to await non-blocking API calls, or 'await TaskEx.Run(...)' to do CPU-bound work on a background thread.

The compiler is actually suggesting this solution:

public class SimplyAwesome : IAmAwesome {
    public async Task MakeAwesomeAsync() {
        await Task.Run(() => {
            // Some really awesome stuff here, but on a BACKGROUND THREAD! WOW!
        });
    }
}

My question is - what should determine when I choose to ignore this compiler warning? In some cases the work is so simple that spawning a thread for it is undeniably counter-productive.

like image 209
Yuck Avatar asked Jan 28 '15 16:01

Yuck


3 Answers

If you really do want to do the work synchronously, you know that your async method will always run synchronously, and that's desirable in the situation, then by all means, ignore the warning. If you understand what the warning is telling you and feel that the action it is describing is correct, then it's not a problem. There's a reason it's a warning and not an error after all.

Of course, the other option is to just not make the method async and to simply use Task.FromResult to return an already completed task instead. It would change the error handling semantics (unless you also catch all exceptions and wrap them into a task that you return) so at least be mindful of that. If you really want exceptions to be propagated through the resulting Task, it may be worth leaving the method async and just suppressing the warning.

like image 146
Servy Avatar answered Oct 30 '22 00:10

Servy


What should determine when I choose to ignore this compiler warning? In some cases the work is so simple that spawning a thread for it is undeniably counter-productive.

The compiler isn't saying "use Task.Run inside this method". It is merely telling you that you prepared him for an async method, adding the async modifier to your method declaration, but you aren't actually awaiting anything.

You could take three approaches:

A. You could ignore the compiler warning and everything will still execute. Note, that there will be a slight overhead of state-machine generation, but your method call will execute synchronously. If the operation is time consuming and might cause the method execution to make a blocking call, this might confuse the users consuming this method.

B. Separate the generation of the "Awesome" into a synchronous interface and an asynchronous interface:

public interface MakeAwesomeAsync
{
    Task MakeAwesomeAsync();
}

public interface MakeAwesome
{
    void MakeAwesome();
}

C. If the operation is not too time consuming, you can simply wrap it in a Task using Task.FromResult. I would definitely measure how long it takes to run the CPU bound operation before choosing this.

like image 37
Yuval Itzchakov Avatar answered Oct 30 '22 00:10

Yuval Itzchakov


As everybody else already pointed out, you have 3 different options, so it's a matter of opinion:

  1. keep it async and ignore the warning
  2. have sync/async overloads
  3. remove async and return a completed task.

I would recommend returning an already completed task:

public class SimplyAwesome : IAmAwesome 
{
    public Task MakeAwesomeAsync() 
    {
        // run synchronously
        return TaskExtensions.CompletedTask;
    }
}

For several reasons:

  • Making a method async has a slight overhead of creating a state-machine and disabling some optimizations (like inlining) because of the try-catch block the compiler adds.
  • You need to deal with the warning and with any other team-member looking at this code wondering where the await is.
  • There's somewhat of a dissonance in marking a completely synchronous method async. It's like adding while(false){} in your code, it will act the same, but it doesn't convey the meaning of the method.

Servy pointed out that returning a task changes the semantics of exception handling. While that's true, I think it's a non-issue.

First of all, most async code calls a method and awaits the returned task at the same place (i.e. await MakeAwesomeAsync()) which means the exception would be thrown at the same place no matter whether the method was async or not.

Second of all, even the .Net framework's Task-returning methods throw exceptions synchronously. Take for example Task.Delay which throws an exception directly without storing it in the returned task, so there's no need to await the task to raise the exception:

try
{
    Task.Delay(-2);
}
catch (Exception e)
{
    Console.WriteLine(e);
}

Since .Net developers need to except to encounter exceptions synchronously in .Net it's reasonable they should also except that from your code.

like image 28
i3arnon Avatar answered Oct 30 '22 02:10

i3arnon