Question

I have a standard, non-async action like:

[HttpPost]
public JsonResult StartGeneratePdf(int id)
{
    PdfGenerator.Current.GenerateAsync(id);
    return Json(null);
}

The idea being that I know this PDF generation could take a long time, so I just start the task and return, not caring about the result of the async operation.

In a default ASP.Net MVC 4 app this gives me this nice exception:

System.InvalidOperationException: An asynchronous operation cannot be started at this time. Asynchronous operations may only be started within an asynchronous handler or module or during certain events in the Page lifecycle. If this exception occurred while executing a Page, ensure that the Page is marked <%@ Page Async="true" %>.

Which is all kinds of irrelevant to my scenario. Looking into it I can set a flag to false to prevent this Exception:

<appSettings>
    <!-- Allows throwaway async operations from MVC Controller actions -->
    <add key="aspnet:AllowAsyncDuringSyncStages" value="true" />
</appSettings>

https://stackoverflow.com/a/15230973/176877
http://msdn.microsoft.com/en-us/library/hh975440.aspx

But the question is, is there any harm by kicking off this Async operation and forgetting about it from a synchronous MVC Controller Action? Everything I can find recommends making the Controller Async, but that isn't what I'm looking for - there would be no point since it should always return immediately.

No correct solution

OTHER TIPS

Relax, as Microsoft itself says (http://msdn.microsoft.com/en-us/library/system.web.httpcontext.allowasyncduringsyncstages.aspx):

This behavior is meant as a safety net to let you know early on if you're writing async code that doesn't fit expected patterns and might have negative side effects.

Just remember a few simple rules:

  • Never await inside (async or not) void events (as they return immediately). Some WebForms Page events support simple awaits inside them - but RegisterAsyncTask is still the highly preferred approach.

  • Don't await on async void methods (as they return immediately).

  • Don't wait synchronously in the GUI or Request thread (.Wait(), .Result(), .WaitAll(), WaitAny()) on async methods that don't have .ConfigureAwait(false) on root await inside them, or their root Task is not started with .Run(), or don't have the TaskScheduler.Default explicitly specified (as the GUI or Request will thus deadlock).

  • Use .ConfigureAwait(false) or Task.Run or explicitly specify TaskScheduler.Default for every background process, and in every library method, that does not need to continue on the synchronization context - think of it as the "calling thread", but know that it is not one (and not always on the same one), and may not even exist anymore (if the Request already ended). This alone avoids most common async/await errors, and also increases performance as well.

Microsoft just assumed you forgot to wait on your task...

UPDATE: As Stephen clearly (pun not intended) stated in his answer, there is an inherit but hidden danger with all forms of fire-and-forget when working with application pools, not solely specific to just async/await, but Tasks, ThreadPool, and all other such methods as well - they are not guaranteed to finish once the request ends (app pool may recycle at any time for a number of reasons).

You may care about that or not (if it's not business-critical as in the OP's particular case), but you should always be aware of it.

The answer turns out to be a bit more complicated:

If what you're doing, as in my example, is just setting up a long-running async task and returning, you don't need to do more than what I stated in my question.

But, there is a risk: If someone expanded this Action later where it made sense for the Action to be async, then the fire and forget async method inside it is going to randomly succeed or fail. It goes like this:

  1. The fire and forget method finishes.
  2. Because it was fired from inside an async Task, it will attempt to rejoin that Task's context ("marshal") as it returns.
  3. If the async Controller Action has completed and the Controller instance has since been garbage collected, that Task context will now be null.

Whether it is in fact null will vary, because of the above timings - sometimes it is, sometimes it isn't. That means a developer can test and find everything working correctly, push to Production, and it explodes. Worse, the error this causes is:

  1. A NullReferenceException - very vague.
  2. Thrown inside .Net Framework code you can't even step into inside of Visual Studio - usually System.Web.dll.
  3. Not captured by any try/catch because the part of the Task Parallel Library that lets you marshal back into existing try/catch contexts is the part that's failing.

So, you'll get a mystery error where things just don't occur - Exceptions are being thrown but you're likely not privy to them. Not good.

The clean way to prevent this is:

[HttpPost]
public JsonResult StartGeneratePdf(int id)
{
    #pragma warning disable 4014 // Fire and forget.
    Task.Run(async () =>
    {
        await PdfGenerator.Current.GenerateAsync(id);
    }).ConfigureAwait(false);

    return Json(null);
}

So, here we have a synchronous Controller with no issues - but to ensure it still won't even if we change it to async later, we explicitly start a new Task via Run, which by default puts the Task on the main ThreadPool. If we awaited it, it would attempt to tie it back to this context, which we don't want - so we don't await it, and that gets us a nuisance warning. We disable the warning with the pragma warning disable.

The InvalidOperationException is not a warning. AllowAsyncDuringSyncStages is a dangerous setting and one that I would personally never use.

The correct solution is to store the request to a persistent queue (e.g., an Azure queue) and have a separate application (e.g., an Azure worker role) processing that queue. This is much more work, but it is the correct way to do it. I mean "correct" in the sense that IIS/ASP.NET recycling your application won't mess up your processing.

If you absolutely want to keep your processing in-memory (and, as a corollary, you're OK with occasionally "losing" reqeusts), then at least register the work with ASP.NET. I have source code on my blog that you can drop in your solution to do this. But please don't just grab the code; please read the entire post so it's clear why this is still not the best solution. :)

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top