G.L SolariaI just thought I would write about a newbie bug that was in a library I was working with recently. I have changed the class names and methods but the concept still applies. Here is a simplified version of the library class:
public class Car
{
public void Start()
{
Console.WriteLine("Beginning start");
StartIgnition();
Console.WriteLine("Ending start");
}
public void Stop()
{
Console.WriteLine("Beginning stop");
StopIgnition();
Console.WriteLine("Ending stop");
}
private async Task StartIgnition()
{
Console.WriteLine("Begin ignition start");
await Task.Delay(1);
Console.WriteLine("Doing more start ignition work");
await Task.Delay(1);
Console.WriteLine("End ignition start");
}
private async Task StopIgnition()
{
Console.WriteLine("Begin stop ignition");
await Task.Delay(1);
Console.WriteLine("Doing more stop ignition work");
await Task.Delay(1);
Console.WriteLine("Ending stop ignition");
}
}
And here is the main program:
class Program
{
static void Main(string[] args)
{
Car car = new Car();
car.Start();
car.Stop();
Console.ReadLine();
}
}
Can you see the problem? Lets look at an example of the program output.
Beginning start
Begin ignition start
Ending start
Beginning stop
Begin stop ignition
Ending stop
Doing more stop ignition work
Doing more start ignition work
Ending stop ignition
End ignition start
If you do a few runs of this class, you may get a different interleaving of messages. By looking at the API, we might have reasonably assumed that all the work to do the start is completed by the time we call stop. But in this run of the program we see that more start ignition work is done after doing more stop ignition work!
The Car class belongs to a library that only exposes a start and stop method that returns void. There is no hint that it executed asynchronously. The implementation of start and stop doesn't await the start or the stop ignition methods. This might result in some of the start work executing after some of the stop work.
Now there is a compiler warning for this:
Warning CS4014 Because this call is not awaited, execution of the current method continues before the call is completed. Consider applying the 'await' operator to the result of the call.
But I have seen suppression of this warning using something like the following:
public static class TaskExtensions
{
// Used to signal that you want to deliberately not await
// for a task to complete and you want to not get the
// compiler warning.
public static void Forget(this Task task) { }
}
Which is used like this:
StartIgnition().Forget();
Importantly it should also be noted that if StartIgnition or StopIgnition raised exceptions, the exception would be lost because exceptions are stored in the task.
Now the problem I encountered was actually buried deep in the library's call hierarchy. I assume the bug I encountered was an evolutionary bug and the code was not initially written this way. It is for this reason many async/await programmers chant the words "async all way". This means you push the async/await all the way up the call hierarchy and leave the top level to decide what to do. The original code could be changed in the following way to adhere to the "async all the way" adage:
public class Car
{
public async Task Start()
{
Console.WriteLine("Beginning start");
await StartIgnition(); // Was: StartIgnition();
Console.WriteLine("Ending start");
}
public async Task Stop()
{
Console.WriteLine("Beginning stop");
await StopIgnition(); // Was: StopIgnition();
Console.WriteLine("Ending stop");
}
private async Task StartIgnition()
{
Console.WriteLine("Begin ignition start");
await Task.Delay(1);
Console.WriteLine("Doing more start ignition work");
await Task.Delay(1);
Console.WriteLine("End ignition start");
}
private async Task StopIgnition()
{
Console.WriteLine("Begin stop ignition");
await Task.Delay(1);
Console.WriteLine("Doing more stop ignition work");
await Task.Delay(1);
Console.WriteLine("Ending stop ignition");
}
}
The above code would allow the main program to decide what to do; It could either await or forget.
I should mention there are times where it is more efficient to return a task and not await it. Consider the following:
public class Car
{
public Task Start()
{
return StartIgnition();
}
public Task Stop()
{
return StopIgnition();
}
private async Task StartIgnition()
{
Console.WriteLine("Begin ignition start");
await Task.Delay(1);
Console.WriteLine("Doing more start ignition work");
await Task.Delay(1);
Console.WriteLine("End ignition start");
}
private async Task StopIgnition()
{
Console.WriteLine("Begin stop ignition");
await Task.Delay(1);
Console.WriteLine("Doing more stop ignition work");
await Task.Delay(1);
Console.WriteLine("Ending stop ignition");
}
}
In the case above, if we were to use "await StartIgnition()", it would cause the compiler to create extra overhead to manage the state machine associated with the async. Instead we can simply return the task. But the downside of this approach is that the compiler won't warn users of Start() and Stop() if they don't await! To counteract this problem, I have seen libraries use the naming convention for all async methods to include the word "Async" (for example, StartAsync() and StopAsync()).
In conclusion, if you find yourself deciding to not await a task-based method, know that you could be laying the groundwork for a nasty future bug.
