2

Suppose I have an Action like below that I want to return the View asap and continue doing some work in the background thread.

public async Task<ActionResult> Index()
{
    Debug.WriteLine("Inside Index");

    var newCustomer = new Customer
    {
        Name = "Ibrahim"
    };

    Task.Run(() => SaveCustomer(newCustomer));

    Debug.WriteLine("Exiting Index");

    return View();
}

private async Task SaveCustomer(Customer NewCustomer)
{
    Debug.WriteLine("Started Saving Customer");

    await Task.Delay(2000);

    Debug.WriteLine("Completed Saving Customer");
}

I do get the output as intended which is:

Inside Index
Exiting Index
Started Saving Customer
Completed Saving Customer

But what bothers me is that I get a warning that my Index action will run synchronously regardless and I should put an await but then the view is returned after SaveCustomer is completed and the purpose is defeated.

How am I doing this wrong? Any suggestion is appreciated.

lbrahim
  • 3,710
  • 12
  • 57
  • 95
  • 1
    maybe a little off-topic - but what if `SaveCustomer(newCustomer)` fails? Shouldn't the current Request be aware of that? – shay__ Sep 05 '15 at 20:44
  • @shay__ Yeah, sure. But this is just an example. I just want to know whether what I want to do is correct or not. – lbrahim Sep 05 '15 at 20:50
  • Can be a logging mechanism for instance rather than saving customer – lbrahim Sep 05 '15 at 21:00

3 Answers3

2

But what bothers me is that I get a warning that my Index action will run synchronously

How am I doing this wrong?

Don't force asynchrony from the top down. Instead, start with naturally-asynchronous operations at the lowest level (e.g., EF6 database access), and allow the asynchrony grow from the lowest-level code upward.

Also, on ASP.NET, you should strongly avoid Task.Run.

Applying these two principles results in an Index method like this:

public async Task<ActionResult> Index()
{
  Debug.WriteLine("Inside Index");

  var newCustomer = new Customer
  {
    Name = "Ibrahim"
  };

  await SaveCustomer(newCustomer);

  Debug.WriteLine("Exiting Index");

  return View();
}

but then the view is returned after SaveCustomer is completed and the purpose is defeated.

Not at all. The purpose of asynchronous ASP.NET code is not to return early to the client. async and await do not change the HTTP protocol. await on the server side yields to the thread pool, not the client.

If you need to return early (and most people don't - they only think they "need" to), then you should use one of the established patterns for returning early (as I describe on my blog). Note that the only proper (i.e., fully reliable) solution entails setting up a reliable queue with an independent background process.

Community
  • 1
  • 1
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • 1
    OP - you should read this answer again and again until you really get it... it's all in there. – shay__ Sep 06 '15 at 04:50
1

Your Index does not make use of any async feature at all. Why did you mark it async? You must be misunderstanding something, not sure what. Remove the async Task specification.

usr
  • 168,620
  • 35
  • 240
  • 369
  • The misunderstanding is that I read somewhere that there are limited reserved threads for HTTP processing so if actions are not async and be blocking then there will be no thread available eventually that is why I marked `Index` as async to begin with. Is it wrong? Maybe not applicable here? – lbrahim Sep 05 '15 at 20:56
  • 1
    Marking it as async does nothing to release a thread. If that was the case all methods would always be async. async pretty much just enables the await keyword. Listen, the chance is 99% that you don't need async IO in any way (under ASP.NET). For decades we have run our servers without async IO, mostly. Make sure you understand what it's for and why before using it. – usr Sep 05 '15 at 21:01
  • 1
    I usually give the following links: http://stackoverflow.com/a/25087273/122718 Why does the EF 6 tutorial use asychronous calls? http://stackoverflow.com/a/12796711/122718 Should we switch to use async I/O by default? – usr Sep 05 '15 at 21:02
  • 1
    I agree with @usr, most chances are you don't need async backend. In my company we refactored the code to be async only when we became large enough, with "too many" customers and 30K+ requests per second. – shay__ Sep 06 '15 at 04:45
1

You get that compiler warning because there is nothing asynchronous in your Index() method. Your Task.Run(() => SaveCustomer(newCustomer)); line means Fire And Forget (non awaited task) - this is very different than asynchronous code. Index() is completely synchronous, while creating a "side Task" to execute sometime in the future. As the other answer mentioned - you could just as well remove the async mark from your method - it's not async.

shay__
  • 3,815
  • 17
  • 34
  • So my `Index` action is actually not blocking and `async` is not needed? – lbrahim Sep 05 '15 at 20:59
  • @lbrahim correct, there is nothing blocking in your **example**. But then again, that's just an example isn't it? Wouldn't your real code make any I/O calls? – shay__ Sep 06 '15 at 04:48