This document has moved to: https://anthonysteele.github.io/AsyncBasicMistakes
I see a lot of code lately that makes some simple mistakes using the async ... await
construct.
- A Task is a promise of a value.
- Most of the time the wait is for network I/O, and there is no thread waiting.
- Avoid
.Result
as much as possible. Let the async flow. - Avoid
Task.Run
. You don't need it. - Avoid
async void
methods
Async can't make your code wait faster, but it can free up threads for greater throughput. However some common antipatterns prevent that from happening.
Think of Task<Order>
as a promise of an order later, i.e. "There will be an order. If I don't have it now, I'll have it at some point in the future". The Computer Science term is "future" or "promise".
Since this is C#, there are a few other things that can happen besides the promised order arriving: The value can arrive, and be null
instead of an order. Or an exception can be thrown instead.
Async ... await
is basically about allowing you to write code in the familiar linear style, but under the hood, it is reconfigured so that the code after the await
completes is put in a callback that is executed later. Without async ... await
we could still write equivalent code using callbacks, but it was just much harder.
This is not unique to C#. Look at how this Javascript promises library works with chained callbacks.
A Task - the promise of a value later - is a general construct which says nothing about how the value is being generated. It's usually network I/O, and there is no thread waiting as there would be for computation.
Most of the time you should not need to use .Result
. Let the async flow. This means that lots of methods have to be sprinkled with async
, Task
and await
: the caller, the caller's caller and so on up the chain. This is just the cost of it, so get used to it. It's not so much an "some async methods" in an app as "an async app".
If you have to use .Result
, use it as few times as high up the call stack as possible.
Ideally, you hand the async Task off to your framework. You can do this in ASP MVC and WebApi as they allow async methods on controllers, you do this in NUnit as tests can be async, in NancyFx, etc.
Calling .Result
forces the code to wait at that point, losing the main advantage that you can hand off whole blocks of code to be executed later when results are available.
The most common exception is for commandline apps, where since Main
cannot be async, you have to do something like this:
class Program
{
static void Main(string[] args)
{
var task = AsyncMain();
task.Wait();
Console.ReadLine();
}
private static async Task AsyncMain()
{
// await the rest of the code
}
}
You really don't need this in most cases (unless you're writing a scheduling engine like the one in JustSaying).
The task returned by an async method will be "hot". i.e. already started and the very heavyweight Task.Run
construct ties up threads and adds nothing of value.
This type is only there for compatibility reasons. When you can, use e.g. public async Task Foo()
instead of public async void Foo()
.
There are patterns for converting code to async. e.g. private bool SomeTest()
becomes private async Task<bool> SomeTest()
. bool
becomes Task<bool>
and void
becomes Task
- and where there is no return value we still need a task so that the caller can know when the async code has completed without a return value. You only drop the Task
when it is necessary for a calling framework that specifically needs a void
return.
- Best Practices in Asynchronous Programming
- Async Await Best Practices Cheat Sheet
- There is no thread.
- What is the promise pattern
This is actual production code:
protected T DoTheThing<T>(Func<T> func)
{
return Task.Run(async () => await DoTheThingAsync(() => Task.FromResult(func()))).Result;
}
As far as I can see,
- There is a method
protected async Task<T> DoTheThingAsync<T>(Func<Task<T>> action)
which wraps theaction
code in timer metrics and error logging. - Someone needed a sync version of
DoTheThingAsync
, and decided that a sync wrapper was the way to do it. First mistake. - When it didn't compile, they just continued piling on code constructs until it compiled, and called it done.
- OK, it probably passed some unit tests as well.
- But it's still a catastrophe. People have learned from this "pattern" that if it doesn't compile, just add
Task.Run
and.Result
until it does, and applied it elsewhere. Unreadable code is poor code. And there is significant unnecessary performance overhead from the heavyweight constructs. - The best way to do this is to actually write a
DoTheThing
method that does the sametry...catch
and timers asDoTheThingAsync
but without the Task and awaits. Sometimes the sync code is best as just a wrapper around the async code, but there are simpler wrappers than the code above, and this is not one of those times.
Best refactor:
// The sync the code to DoTheThingAsync,
// i.e. without the awaits
// this avoids the overhead and confusion of creating tasks and syncing them up again
protected T DoTheThing<T>(Func<T> func)
{
T result = default(T);
var timer = Stopwatch.StartNew();
try
{
result = func();
timer.Stop;
LogElapsedTime(timer.Elapsed);
return result;
}
catch (Exception ex)
{
_logger.Error("DoTheThing failed", ex);
throw;
}
}
if we want to run something in multithread not async and the method need not be return
then shall we use following code