Async ASP.NET MVC 5 controller method

Question

I'm attempting to correctly convert a synchronous controller method to asynchronous, given that the operation it performs is CPU-intensive.

private MyDbContext db = new MyDbContext();
public async Task<ActionResult> Index()
{
    List<DashboardItemViewModel> viewModels = new List<DashboardItemViewModel>();
    List<ProductModel> products = db.Products.ToList();
    await Task.Run(() =>
    {
        foreach (ClientModel client in db.Clients)
        {
            // Constructor uses lots of logic, mostly looping through the list
            //   many times
            viewModels.Add(new DashboardItemViewModel(client, products));
        }
    });
    return View(viewModels);
}

Answer 1

I think the closures are unnecessary and create (small or not) overhead you can avoid. See Using Asynchronous Methods in ASP.NET MVC 4 where specifically the placement of the await keyword is within the return and additionally (unlike the example) you can return the entire self contained code block.

Answer 2

As a key area of concern with this code, you really should be passing a cancellation token here, as it is a long running operation.

You need to worry about the client giving up on the request, let's say they navigated to the URL and decided they didn't mean to. The operation will still continue to run until the thread operation is complete.

This kind of issue can also lead to all kinds of problems.

Note that this can also occur when there multiple requests (say an impatient user hits refresh frequently) or a lot of traffic.