7

Suppose I have an interface which includes an async method, and I have two different implementations of that interface. One of the two implementations is naturally async, and the other is not. What would be the "most correct" way of implementing the non-async method?

public interface ISomething {
    Task<Foo> DoSomethingAsync();
}

// Normal async implementation
public class Implementation1 : ISomething {
    async Task<Foo> ISomething.DoSomethingAsync() {
        return await DoSomethingElseAsync();
    }
}

// Non-async implementation
public class Implementation2 : ISomething {
    // Should it be:
    async Task<Foo> ISomething.DoSomethingAsync() {
        return await Task.Run(() => DoSomethingElse());
    }
    // Or:
    async Task<Foo> ISomething.DoSomethingAsync() {
        return DoSomethingElse();
    }
}

I try to keep up with Stephen Cleary's blog, and I know neither one of these actually provides any async benefits, and I'm ok with that. The second one seems more correct to me, since it doesn't pretend to be something it's not, but it does give a compiler warning, and those add up and get distracting.

This would all be inside ASP.NET (both web MVC and WebAPI), if that makes a difference.

Joe Enos
  • 39,478
  • 11
  • 80
  • 136
  • Your "Normal async implementation" is not valid... you have nothing returning, yet your return type is `Task`. – B.K. Mar 08 '15 at 17:59
  • @B.K. Thanks, I forgot the `return` in there. Fixed. – Joe Enos Mar 08 '15 at 18:01
  • You can also use `Task.RunSynchronously` instead of `Task.Run` to have the `async`-style exception propagation behavior without redundant `ThreadPool` offloading, as described [here](http://stackoverflow.com/a/21082631/1768303). – noseratio Mar 09 '15 at 03:57

2 Answers2

8

You can forgo the async modifier altogether and use Task.FromResult to return a completed task synchronously:

Task<Foo> ISomething.DoSomethingAsync()
{
    return Task.FromResult(DoSomethingElse());
}

This takes care of the warning and has better performance as it doesn't need the state machine overhead of an async method.

However, this does change the semantics of exception handling a bit. If that's an issue then you should use the synchronous async method approach and accept the warning (or turn it off with a comment):

#pragma warning disable 1998
    async Task<Foo> ISomething.DoSomethingAsync() 
#pragma warning restore 1998
    {
        return DoSomethingElse();
    }

As Stephen Cleary suggested you can also take care of that warning (while keeping the method synchronous) by awaiting an already completed task:

async Task<Foo> ISomething.DoSomethingAsync() 
{
    await Task.FromResult(false); // or Task.CompletedTask in .Net 4.6
    return DoSomethingElse();
}
Community
  • 1
  • 1
i3arnon
  • 113,022
  • 33
  • 324
  • 344
  • 1
    Or an async noop: `await Task.CompletedTask;` in .NET 4.6. – Stephen Cleary Mar 08 '15 at 17:45
  • @StephenCleary To get rid of the warning? Isn't that hacky? – i3arnon Mar 08 '15 at 17:46
  • Cool, thanks. I'll be calling it with `await`, so I didn't really think about not putting `async` on the method. I'll consider both approaches - sounds like either one would work for me. – Joe Enos Mar 08 '15 at 17:58
  • @i3arnon: Yeah, it's hacky. But I'm not a big fan of `#pragma`, either. – Stephen Cleary Mar 08 '15 at 18:13
  • One other way without `async` but with similar exception propagation is `var task = new Task(() => DoSomethingElse()); task.RunSynchronously(TaskScheduler.Default); return task;` @StephenCleary, does it make sense vs `await Task.CompletedTask`? – noseratio Mar 09 '15 at 04:05
  • 1
    @Noseratio: I'd be wary of `RunSynchronously(Default)`; it may queue the work to a background thread in this scenario, I'm not actually sure. – Stephen Cleary Mar 09 '15 at 11:56
  • @StephenCleary, I'd be interested to learn more about the edge cases. To my experience, `ThreadPoolTaskScheduler.TryExecuteTaskInline` only queues `RunSynchronously` to `ThreadPool` when the current thread's stack is too deep (which IMO makes sense). – noseratio Mar 10 '15 at 03:49
2

It really depends on what your method is doing:

  • no I/O, neglible amount of cpu work
  • cpu intensive work
  • I/O intensive work

no I/O, neglible amount of cpu work

You should compute the result synchronously and create a Task holding the result.

Task<Foo> ISomething.DoSomethingAsync() {
    Foo result;
    // do something not hurting performance
    // no I/O here
    return Task.FromResult(result);
}

Note however that any exception will be thrown when the method is called, not when the task is awaited. For the latter case, which is compliant to the other types of work, you should use async nonetheless:

async Task<Foo> ISomething.DoSomethingAsync() {
    Foo result;
    // do something not hurting performance
    // no I/O here
    return result;
}

cpu intensive work

You should start a Task with Task.Run and do the cpu intensive work in the task.

Task<Foo> ISomething.DoSomethingAsync() {
    return Task.Run(() => {
        Foo result;
        // do some CPU intensive work here
        return result;
    });
}

I/O intensive work

You should use the async keyword and await any async I/O method. Do not use synchronous I/O methods.

async Task<Foo> ISomething.DoSomethingAsync() {
    Stream s = new ....

    // or any other async I/O operation
    var data = await s.ReadToEndAsync(); 

    return new Foo(data);
}
Oliver Hanappi
  • 12,046
  • 7
  • 51
  • 68