13

Recently I've developed doubts about the way I'm implementing the async-await pattern in my Web API projects. I've read that async-await should be "all the way" and that's what I've done. But it's all starting to seem redundant and I'm not sure that I'm doing this correctly. I've a got a controller that calls a repository and it calls a data access (entity framework 6) class - "async all the way". I've read a lot of conflicting stuff on this and would like to get it cleared-up.

EDIT: The referenced possible duplicate is a good post, but not specific enough for my needs. I included code to illustrate the problem. It seems really difficult to get a decisive answer on this. It would be nice if we could put async-await in one place and let .net handle the rest, but we can't. So, am I over doing it or is it not that simple.

Here's what I've got:

Controller:

public async Task<IHttpActionResult> GetMessages()
{
    var result = await _messageRepository.GetMessagesAsync().ConfigureAwait(false);
    return Ok(result);
}

Repository:

public async Task<List<string>> GetMessagesAsync()    
{
    return await _referralMessageData.GetMessagesAsync().ConfigureAwait(false);
}

Data:

public async Task<List<string>> GetMessagesAsync()
{           
    return await _context.Messages.Select(i => i.Message).ToListAsync().ConfigureAwait(false);
}
Big Daddy
  • 5,160
  • 5
  • 46
  • 76
  • 3
    @SpacemanSpiff: Subjectivity is not a valid criteria. If it's too subjective for Stack Overflow, it's probably too subjective for Programmers as well. Read http://programmers.stackexchange.com/help/on-topic to find out what is actually on topic there. – Robert Harvey Jul 13 '15 at 18:30
  • @SpacemanSpiff...I don't think this is an opinion-based question. It's all about structuring the code. The documentation and posts on this are conflicting and the ramifications can be considerable if not done properly. My guess is there's a lot of poor async-await implementations out there. Thanks. – Big Daddy Jul 13 '15 at 18:31
  • possible duplicate of [When I should use Async Controllers in ASP.NET MVC](http://stackoverflow.com/questions/30566848/when-i-should-use-async-controllers-in-asp-net-mvc) – Fabjan Jul 13 '15 at 18:32
  • @Fabjan...it's a good post and I saw it already. I included code in my question to make it specific. Why is it so hard to get this cleared-up? – Big Daddy Jul 13 '15 at 18:38
  • 2
    In truth, I'm not at all sure what you're asking here. Can you make your question more specific? – Robert Harvey Jul 13 '15 at 18:38
  • 1
    @BigDaddy Maybe you should want to take a look at this question: [Async all the way down?](http://stackoverflow.com/questions/12016322/async-all-the-way-down) and [Async all the way down issue](http://stackoverflow.com/questions/21072205/async-all-the-way-down-issue) (It seems that your pattern is called the `return await` pattern) – Matias Cicero Jul 13 '15 at 18:43
  • You need to specify what sources are confusing and seem conflicting to you. I don't know this pattern, but do you really need to use await all the time? especially in the repository? – Luminous Jul 13 '15 at 18:47
  • @BigDaddy Not sure your edit to the code part makes it more readable than mine. – Hossein Narimani Rad Jul 13 '15 at 18:48
  • @RobertHarvey...I read quite a bit about this and have implemented it successfully in production, but I'm not convinced that I'm doing it right. For this code, do I need to do async-await "all the way" as many have suggested - Stephen Cleary, etc? Is it redundant? Is it really good? I'm not sure anymore. 6 months ago I would have said this is the way to go. – Big Daddy Jul 13 '15 at 18:52
  • 1
    Just to bring more conflicting information to the table here are two very contrarian pieces of mine: http://stackoverflow.com/a/25087273/122718, http://stackoverflow.com/a/12796711/122718. In essence they say that async is almost always a waste of time in ASP.NET. Important: I actually say *why* instead of just stating some dogma. Be aware that async is all the rage right now and often advice is stated without reason. – usr Jul 13 '15 at 18:52
  • @HosseinNarimaniRad...no disrespect intended. I think it's good, just different from mine. – Big Daddy Jul 13 '15 at 18:53
  • @usr..thanks for adding to my confusion! What did you mean by this sentence - "await makes 99% of the cases (almost) as simple as synchronous code"? Thanks. – Big Daddy Jul 13 '15 at 19:02
  • @BigDaddy have you ever used the APM pattern for async IO? It's nasty. await almost closes the gap. – usr Jul 13 '15 at 19:08
  • I did and slowly gravitated away from it - a real pain. Am I going to be doing the same with await? – Big Daddy Jul 13 '15 at 19:15
  • 2
    @BigDaddy I advise to use async IO in the sweet spot places (see my posts for where that is). It's a boon there and a hindrance everywhere else. Understand where and why async IO helps. It is amazing what cumulative amount of pain developers are bring onto themselves with async IO while not noticing that they do not derive *any* benefit from it. Programmers are supposed to be logical beings! :) – usr Jul 13 '15 at 19:39
  • @usr...your comments and previous posts are very helpful – Big Daddy Jul 14 '15 at 13:16

2 Answers2

12

It would be nice if we could put async-await in one place and let .net handle the rest, but we can't. So, am I over doing it or is it not that simple.

It would be nice if it was simpler.

The sample repository and data code don't have much real logic in them (and none after the await), so they can be simplified to return the tasks directly, as other commenters have noted.

On a side note, the sample repository suffers from a common repository problem: doing nothing. If the rest of your real-world repository is similar, you might have one level of abstraction too many in your system. Note that Entity Framework is already a generic unit-of-work repository.

But regarding async and await in the general case, the code often has work to do after the await:

public async Task<IHttpActionResult> GetMessages()
{
  var result = await _messageRepository.GetMessagesAsync();
  return Ok(result);
}

Remember that async and await are just fancy syntax for hooking up callbacks. There isn't an easier way to express this method's logic asynchronously. There have been some experiments around, e.g., inferring await, but they have all been discarded at this point (I have a blog post describing why the async/await keywords have all the "cruft" that they do).

And this cruft is necessary for each method. Each method using async/await is establishing its own callback. If the callback isn't necessary, then the method can just return the task directly, avoiding async/await. Other asynchronous systems (e.g., promises in JavaScript) have the same restriction: they have to be asynchronous all the way.

It's possible - conceptually - to define a system in which any blocking operation would yield the thread automatically. My foremost argument against a system like this is that it would have implicit reentrancy. Particularly when considering third-party library changes, an auto-yielding system would be unmaintainable IMO. It's far better to have the asynchrony of an API explicit in its signature (i.e., if it returns Task, then it's asynchronous).

Now, @usr makes a good point that maybe you don't need asynchrony at all. That's almost certainly true if, e.g., your Entity Framework code is querying a single instance of SQL Server. This is because the primary benefit of async on ASP.NET is scalability, and if you don't need scalability (of the ASP.NET portion), then you don't need asynchrony. See the "not a silver bullet" section in my MSDN article on async ASP.NET.

However, I think there's also an argument to be made for "natural APIs". If an operation is naturally asynchronous (e.g., I/O-based), then its most natural API is an asynchronous API. Conversely, naturally synchronous operations (e.g., CPU-based) are most naturally represented as synchronous APIs. The natural API argument is strongest for libraries - if your repository / data access layer was its own dll intended to be reused in other (possibly desktop or mobile) applications, then it should definitely be an asynchronous API. But if (as is more likely the case) it is specific to this ASP.NET application which does not need to scale, then there's no specific need to make the API either asynchronous or synchronous.

But there's a good two-pronged counter-argument regarding developer experience. Many developers don't know their way around async at all; would a code maintainer be likely to mess it up? The other prong of that argument is that the libraries and tooling around async are still coming up to speed. Most notable is the lack of a causality stack when there are exceptions to trace down (on a side note, I wrote a library that helps with this). Furthermore, parts of ASP.NET are not async-compatible - most notably, MVC filters and child actions (they are fixing both of those with ASP.NET vNext). And ASP.NET has different behavior regarding timeouts and thread aborts for asynchronous handlers - adding yet a little more to the async learning curve.

Of course, the counter-counter argument would be that the proper response to behind-the-times developers is to train them, not restrict the technologies available.

In short:

  • The proper way to do async is "all the way". This is especially true on ASP.NET, and it's not likely to change anytime soon.
  • Whether async is appropriate, or helpful, is up to you and your application's scenario.
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • 3
    Dev experience: You can't profile async IO either. Nothing is on the stack. Nobody seems to be doing anything. – usr Jul 13 '15 at 19:38
  • 1
    Also don't forget localisation: async tasks can be picked up by a different thread, and the UI culture of the thread could then be different. We had issues with this with a system that was accessed from multiple countries with localisation. – NibblyPig Jul 14 '15 at 12:25
  • Thanks for your insightful comments. But I'm still confused. You write "If the callback isn't necessary, then the method can just return the task directly, avoiding async/await" and then you write "The proper way to do async is "all the way"". These seem like conflicting statements to me. Can I leave my controller async and have the repository and data classes return Task<>? Please clarify. – Big Daddy Jul 14 '15 at 13:02
  • When I use the phrase "async all the way", I mean each method calling an asynchronous method should be asynchronous. This is "asynchronous" as in "Task-returning", not "uses the async keyword". The async keyword is an implementation detail, and it's only one way to define a Task-returning method. Returning the Task directly is another way to define a Task-returning method. Other approaches include `TaskFactory.FromAsync` and `TaskCompletionSource`. – Stephen Cleary Jul 14 '15 at 14:55
7
public async Task<List<string>> GetMessagesAsync()    
{
    return await _referralMessageData.GetMessagesAsync().ConfigureAwait(false);
}

public async Task<List<string>> GetMessagesAsync()
{           
    return await _context.Messages.Select(i => i.Message).ToListAsync().ConfigureAwait(false);
}

If the only calls you do to asynchronous methods are tail-calls, then you don't really need to await:

public Task<List<string>> GetMessagesAsync()    
{
    return _referralMessageData.GetMessagesAsync();
}

public Task<List<string>> GetMessagesAsync()
{           
    return _context.Messages.Select(i => i.Message).ToListAsync();
}

About the only thing you lose is some stack-trace information, but that's rarely all that useful. Remove the await then instead of generating a state-machine that handles the waiting you just pass back the task produced by the called method up to the calling method, and the calling method can await on that.

The methods can also sometimes be inlined now, or perhaps have tail-call optimisation done on them.

I'd even go so far as to turn non-task-based paths into task-based if it was relatively simple to do so:

public async Task<List<string>> GetMeesagesAsync()
{
   if(messageCache != null)
     return messageCache;
   return await _referralMessageData.GetMessagesAsync().ConfigureAwait(false);
}

Becomes:

public Task<List<string>> GetMeesagesAsync()
{
   if(messageCache != null)
     return Task.FromResult(messageCache);
   return _referralMessageData.GetMessagesAsync();
}

However, if at any point you need the results of a task to do further work, then awaiting is the way to go.

Jon Hanna
  • 110,372
  • 10
  • 146
  • 251
  • Excellent explaination. – Big Daddy Jul 14 '15 at 13:04
  • 2
    Worth noting that this is still async all the way down, it's just not `async` all the way down ;) – Jon Hanna Jul 14 '15 at 14:22
  • So async "all the way down" means calling methods with return types of Task<>. It's not necessary to have async Task<>. Am I correctly understanding you? I am guessing that the top-level method, in my case the controller method, should be async/await because it is waiting for the other methods to return data. – Big Daddy Jul 14 '15 at 14:42
  • Yes, `async` and `await` are a means to the end, but the end is a `Task<>` that yields the processor at appropriate times, and sometimes it's simpler without them. However, sometimes it's **much** simpler with them. For your controller you could write something that creates an awaitable `Task` to return another way, but `await`ing on `.GetMessagesAsync()` and then creating the `ActionResult` from that is both a very simple way of doing so and one that deals with a lot of the potential complications easily. – Jon Hanna Jul 14 '15 at 14:50
  • Your answer and Stephen Cleary's are both excellent. I like hat you included code in yours. Awarding the accepted answer is a difficult decision, but I'm giving this one the nod. – Big Daddy Jul 15 '15 at 11:21