Common pitfall with async in C#

Just a quick rant today... :)

I love how easy it is to write asynchronous code with Tasks and the async and await keywords in C#. One problem that I keep running into is this kind of thing in code examples on line.

public static T Value<T>(this HttpContent content)
{
    var task = content.ReadAsAsync<T>();
    task.Wait();
    return task.Result;
}

Now there's a couple of problems with the above code. Whoever wrote it obviously thought it was going to do something magic, after all - they've chosen to use the ReadAsAsync<T>() method! Unfortunately, the very next line calls Wait() on the returned Task. So the Thread that could have been doing other work while the content was being read is just blocking anyway. I.e. there is no benefit at all to using the Async code.
There's also the fact that accessing task.Result would have blocked anyway meaning the Wait() was pointless.

This code was checked in with the comment "Repopulate Async so no one has to wait!". I'm not intentionally tearing someone down - it was just a good example of misunderstanding asynchronous code. The intention was laudable, a long running non-cpu bound web api action? Prime candidate for asynchronous and credit to the guy for noticing that.

So what was the solution? 

Remove the pointless extension method (only used once) and await ReadAsync in the asynchronous action that was using it.

Don't just use async methods for the sake of it, use them when something higher up the call chain can actually take advantage of it!

Comments

Popular posts from this blog

Trimming strings in action parameters in ASP.Net Web API

Full text search in Entity Framework 6 using command interception

Composing Expressions in C#