View on GitHub

anthonysteele.github.io

Avoiding simple mistakes in async await

I see a lot of code lately that makes some simple mistakes using the async ... await construct in C# code.

TL;DR

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.

A task is a promise

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. And futures and promises in Java.

There is no thread

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.

Avoid taking the result

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. The model is 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. I generally do this while refactoring out .Result calls as it’s a step towards the goal.

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.

I heard Kathleen Dollard compare the async call stack to a Light tube: Any blockage between the basement and roof will prevent light getting through. And with async, any blocking code will prevent the task from getting through.

Exceptions to that rule

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
	}
 }  

Avoid Task.Run

Some people have the idea that Task.Run is necessary or even good for using async and await. It is not.

In async code, you do not need Task.Run to start a task since The task returned by an async method will be “hot”. i.e. already started. The very heavyweight Task.Run construct ties up threads and adds nothing of value.

I have seen cases where the calling code was not async, and using .Result to exit the async code caused a deadlock. Then Task.Run was used and worked. But don’t mistake this for the right thing - it is an ugly hack with a large performance penalty, and you should far rather look at propagating the Task up the call stack to where the framework can handle it. The fact that you get a deadlock probably means that you have other problems lurking.

Code that is never async or code that is always async tends to not have these problems. Code that tries to be both often does.

Avoid async void methods

The type async void 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.

In async code, you should await wherever possible

I asked the stupid question so that you don’t have to: In server code, if you use one await for a slow operation, should you use a second await for a fast operation?. And the answer is “yes”. If you have one await in your code, then you might as well use as many as you can to chop the operation up into small parts so that operations can flow through better. So if your code can trivially await an async method instead of a calling the sync version, then you should do so. There is no additional performance penalty to this.

In UI code, there is a guideline that “operations that could take over 50ms should be async” so as to not lock the UI thread. But it’s clear that this does not apply to web server code where it’s all thread pool threads already. Even quicker operations should also be awaited in order to break code into smaller chunks and so increase throughput.

Further reading

The catastrophe

This is actual production code:

 protected T ExecWithLoggingAndTiming<T>(Func<T> func)
 {
        return Task.Run(async () => await ExecWithLoggingAndTimingAsync(() => Task.FromResult(func()))).Result;
 }

As far as I can see:

Best refactor:

 // The sync version of the code to ExecWithLoggingAndTimingAsync, 
 // i.e. without the awaits 
 // this avoids the overhead and confusion of creating tasks and syncing them up again
 protected T ExecWithLoggingAndTiming<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("ExecWithLoggingAndTiming failed", ex);
     throw;
   }
 }