116

I have a multi-tier .Net 4.5 application calling a method using C#'s new async and await keywords that just hangs and I can't see why.

At the bottom I have an async method that extents our database utility OurDBConn (basically a wrapper for the underlying DBConnection and DBCommand objects):

public static async Task<T> ExecuteAsync<T>(this OurDBConn dataSource, Func<OurDBConn, T> function)
{
    string connectionString = dataSource.ConnectionString;

    // Start the SQL and pass back to the caller until finished
    T result = await Task.Run(
        () =>
        {
            // Copy the SQL connection so that we don't get two commands running at the same time on the same open connection
            using (var ds = new OurDBConn(connectionString))
            {
                return function(ds);
            }
        });

    return result;
}

Then I have a mid level async method that calls this to get some slow running totals:

public static async Task<ResultClass> GetTotalAsync( ... )
{
    var result = await this.DBConnection.ExecuteAsync<ResultClass>(
        ds => ds.Execute("select slow running data into result"));

    return result;
}

Finally I have a UI method (an MVC action) that runs synchronously:

Task<ResultClass> asyncTask = midLevelClass.GetTotalAsync(...);

// do other stuff that takes a few seconds

ResultClass slowTotal = asyncTask.Result;

The problem is that it hangs on that last line forever. It does the same thing if I call asyncTask.Wait(). If I run the slow SQL method directly it takes about 4 seconds.

The behaviour I'm expecting is that when it gets to asyncTask.Result, if it's not finished it should wait until it is, and once it is it should return the result.

If I step through with a debugger the SQL statement completes and the lambda function finishes, but the return result; line of GetTotalAsync is never reached.

Any idea what I'm doing wrong?

Any suggestions to where I need to investigate in order to fix this?

Could this be a deadlock somewhere, and if so is there any direct way to find it?

Liam
  • 27,717
  • 28
  • 128
  • 190
Keith
  • 150,284
  • 78
  • 298
  • 434

5 Answers5

167

Yep, that's a deadlock all right. And a common mistake with the TPL, so don't feel bad.

When you write await foo, the runtime, by default, schedules the continuation of the function on the same SynchronizationContext that the method started on. In English, let's say you called your ExecuteAsync from the UI thread. Your query runs on the threadpool thread (because you called Task.Run), but you then await the result. This means that the runtime will schedule your "return result;" line to run back on the UI thread, rather than scheduling it back to the threadpool.

So how does this deadlock? Imagine you just have this code:

var task = dataSource.ExecuteAsync(_ => 42);
var result = task.Result;

So the first line kicks off the asynchronous work. The second line then blocks the UI thread. So when the runtime wants to run the "return result" line back on the UI thread, it can't do that until the Result completes. But of course, the Result can't be given until the return happens. Deadlock.

This illustrates a key rule of using the TPL: when you use .Result on a UI thread (or some other fancy sync context), you must be careful to ensure that nothing that Task is dependent upon is scheduled to the UI thread. Or else evilness happens.

So what do you do? Option #1 is use await everywhere, but as you said that's already not an option. Second option which is available for you is to simply stop using await. You can rewrite your two functions to:

public static Task<T> ExecuteAsync<T>(this OurDBConn dataSource, Func<OurDBConn, T> function)
{
    string connectionString = dataSource.ConnectionString;

    // Start the SQL and pass back to the caller until finished
    return Task.Run(
        () =>
        {
            // Copy the SQL connection so that we don't get two commands running at the same time on the same open connection
            using (var ds = new OurDBConn(connectionString))
            {
                return function(ds);
            }
        });
}

public static Task<ResultClass> GetTotalAsync( ... )
{
    return this.DBConnection.ExecuteAsync<ResultClass>(
        ds => ds.Execute("select slow running data into result"));
}

What's the difference? There's now no awaiting anywhere, so nothing being implicitly scheduled to the UI thread. For simple methods like these that have a single return, there's no point in doing an "var result = await...; return result" pattern; just remove the async modifier and pass the task object around directly. It's less overhead, if nothing else.

Option #3 is to specify that you don't want your awaits to schedule back to the UI thread, but just schedule to the thread pool. You do this with the ConfigureAwait method, like so:

public static async Task<ResultClass> GetTotalAsync( ... )
{
    var resultTask = this.DBConnection.ExecuteAsync<ResultClass>(
        ds => return ds.Execute("select slow running data into result");

    return await resultTask.ConfigureAwait(false);
}

Awaiting a task normally would schedule to the UI thread if you're on it; awaiting the result of ContinueAwait will ignore whatever context you are on, and always schedule to the threadpool. The downside of this is you have to sprinkle this everywhere in all functions your .Result depends on, because any missed .ConfigureAwait might be the cause of another deadlock.

Jason Malinowski
  • 18,148
  • 1
  • 38
  • 55
  • 9
    BTW, the question is about ASP.NET, so there is no UI thread. But the issue with deadlocks is exactly the same, because of the ASP.NET `SynchronizationContext`. – svick Jan 25 '13 at 17:37
  • 1
    That explained a lot, as I had similar .Net 4 code that didn't have the problem but that used the TPL without the `async`/`await` keywords. – Keith Jan 25 '13 at 18:33
  • 5
    TPL = Task Parallel Library https://msdn.microsoft.com/en-us/library/dd460717(v=vs.110).aspx – Jamie Ide Dec 21 '15 at 19:33
  • 1
    If anyone is looking for the VB.net code (like me) it is explained here: https://learn.microsoft.com/en-us/dotnet/visual-basic/programming-guide/concepts/async/index – MichaelDarkBlue Jan 04 '19 at 13:59
  • 1
    Can you please help me in https://stackoverflow.com/questions/54360300/bulk-read-images-from-url-and-then-process-threads-get-stuck-hang-somewhere – Jitendra Pancholi Jan 25 '19 at 06:53
  • “you don't want your awaits to schedule back to the UI thread, but just schedule to the UI thread”, Can you clarify the distinction between “back to the UI thread” and “to the UI thread”? – Jace May 13 '20 at 12:19
  • 1
    @Jace: fixed, one of those "UI thread"s was supposed to be "thread pool". – Jason Malinowski May 14 '20 at 02:07
  • UI thread, that's the catch. I have an async _PatientFolderDataService.Init().Result_ that works as a charm in a NUnit test [Setup] **we cannot await inside Fixture and Setup Nunit**, and same method called as _await PatientFolderDataService.Init()_ injected in two Presenters of 2 different ViewForms; in one works and in other, deadlock. – Marcelo Scofano Diniz Jan 08 '22 at 22:11
40

This is the classic mixed-async deadlock scenario, as I describe on my blog. Jason described it well: by default, a "context" is saved at every await and used to continue the async method. This "context" is the current SynchronizationContext unless it it null, in which case it is the current TaskScheduler. When the async method attempts to continue, it first re-enters the captured "context" (in this case, an ASP.NET SynchronizationContext). The ASP.NET SynchronizationContext only permits one thread in the context at a time, and there is already a thread in the context - the thread blocked on Task.Result.

There are two guidelines that will avoid this deadlock:

  1. Use async all the way down. You mention that you "can't" do this, but I'm not sure why not. ASP.NET MVC on .NET 4.5 can certainly support async actions, and it's not a difficult change to make.
  2. Use ConfigureAwait(continueOnCapturedContext: false) as much as possible. This overrides the default behavior of resuming on the captured context.
Dongdong
  • 2,208
  • 19
  • 28
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • Does `ConfigureAwait(false)` guarantee that the current function resumes on a different context? – chue x Jan 25 '13 at 18:38
  • The MVC framework supports it, but this is part of an existing MVC app with lots of client side JS already present. I can't easily switch to an `async` action without breaking the way this works client side. I certainly plan to investigate that option longer term though. – Keith Jan 25 '13 at 18:42
  • Just to clarify my comment - I was curious if using `ConfigureAwait(false)` down the call tree would have solved the OP's problem. – chue x Jan 25 '13 at 18:45
  • 3
    @Keith: Making an MVC action `async` does not affect the client side at all. I explain this in another blog post, [`async` Doesn't Change the HTTP Protocol](http://nitoprograms.blogspot.com/2012/08/async-doesnt-change-http-protocol.html). – Stephen Cleary Jan 25 '13 at 18:52
  • @chuex: `ConfigureAwait(false)` will execute the continuation without re-entering the captured context. It doesn't actually *guarantee* that the continuation will run on the thread pool thread, though this is what happens in almost every situation (including this one). And yes, *either* of the guidelines I posted will prevent the deadlock. They are *both* recommended. :) – Stephen Cleary Jan 25 '13 at 18:56
  • @StephenCleary I have a fairly complex base controller class and the method returns `ActionResult` with HTML that loads in the page - I can't just switch that to `async Task`. Am I missing something? – Keith Jan 28 '13 at 10:25
  • 1
    @Keith: It's normal for `async` to "grow" through the codebase. If your controller method may depend on asynchronous operations, then the base class method *should* return `Task`. Transitioning a large project to `async` is always awkward because mixing `async` and sync code is difficult and tricky. Pure `async` code is much simpler. – Stephen Cleary Jan 28 '13 at 13:49
  • @StephenCleary in this case I have another problem - I'm on .Net 4.5 but the web project is MVC 3 (not 4), which makes it a lot more messy to have asynchronous actions. I'm investigating a move to MVC 4 in the long run, it looks like `async` actions will be a lot easier when I do so. – Keith Jan 28 '13 at 14:00
  • @Keith: `async` is much easier with MVC4, and even easier with WebAPI. With MVC3 you'd need to use `AsyncController` and probably [expose `async` methods as APM](http://msdn.microsoft.com/en-us/library/hh873178.aspx#TapToApm). – Stephen Cleary Jan 28 '13 at 14:29
17

I was in the same deadlock situation but in my case calling an async method from a sync method, what works for me was:

private static SiteMetadataCacheItem GetCachedItem()
{
      TenantService TS = new TenantService(); // my service datacontext
      var CachedItem = Task.Run(async ()=> 
               await TS.GetTenantDataAsync(TenantIdValue)
      ).Result; // dont deadlock anymore
}

is this a good approach, any idea?

Danilow
  • 390
  • 3
  • 9
5

Just to add to the accepted answer (not enough rep to comment), I had this issue arise when blocking using task.Result, event though every await below it had ConfigureAwait(false), as in this example:

public Foo GetFooSynchronous()
{
    var foo = new Foo();
    foo.Info = GetInfoAsync.Result;  // often deadlocks in ASP.NET
    return foo;
}

private async Task<string> GetInfoAsync()
{ 
    return await ExternalLibraryStringAsync().ConfigureAwait(false);
}

The issue actually lay with the external library code. The async library method tried to continue in the calling sync context, no matter how I configured the await, leading to deadlock.

Thus, the answer was to roll my own version of the external library code ExternalLibraryStringAsync, so that it would have the desired continuation properties.


wrong answer for historical purposes

After much pain and anguish, I found the solution buried in this blog post (Ctrl-f for 'deadlock'). It revolves around using task.ContinueWith, instead of the bare task.Result.

Previously deadlocking example:

public Foo GetFooSynchronous()
{
    var foo = new Foo();
    foo.Info = GetInfoAsync.Result;  // often deadlocks in ASP.NET
    return foo;
}

private async Task<string> GetInfoAsync()
{ 
    return await ExternalLibraryStringAsync().ConfigureAwait(false);
}

Avoid the deadlock like this:

public Foo GetFooSynchronous
{
    var foo = new Foo();
    GetInfoAsync()  // ContinueWith doesn't run until the task is complete
        .ContinueWith(task => foo.Info = task.Result);
    return foo;
}

private async Task<string> GetInfoAsync
{
    return await ExternalLibraryStringAsync().ConfigureAwait(false);
}
  • What's the downvote for? This solution is working for me. – Cameron Jeffers Dec 22 '15 at 19:40
  • You're returning the object before the `Task` has been completed, and providing the caller no means of determining when the mutation of the returned object actually happens. – Servy Dec 22 '15 at 19:40
  • hmm yeah I see. So should I expose some sort of "wait until task completes" method that uses a manually blocking while loop (or something like that)? Or pack such a block into the `GetFooSynchronous` method? – Cameron Jeffers Dec 22 '15 at 19:46
  • 1
    If you do, it'll deadlock. You need to async all the way up by returning a `Task` instead of blocking. – Servy Dec 22 '15 at 19:46
  • Unfortunately that's not an option, the class implements a synchronous interface that I can't change. – Cameron Jeffers Dec 22 '15 at 19:49
  • What I don't understand is why `ConfigureAwait(false)` doesn't work in this instance. Could it have something to do with the async operation I'm wating for making HTTP network calls and messing with ASP.NET's `HttpContext`? – Cameron Jeffers Dec 22 '15 at 19:55
  • If `ExternalLibraryStringAsync` post continuations to the current sync context before completing itself then it'd deadlock if you synchronously wait on it from the current sync context. – Servy Dec 22 '15 at 20:01
  • Ok I rolled my own version of the external code, seems to be working now. Thanks a ton @Servy ! – Cameron Jeffers Dec 22 '15 at 20:52
2

quick answer : change this line

ResultClass slowTotal = asyncTask.Result;

to

ResultClass slowTotal = await asyncTask;

why? you should not use .result to get the result of tasks inside most applications except console applications if you do so your program will hang when it gets there

you can also try the below code if you want to use .Result

ResultClass slowTotal = Task.Run(async ()=>await asyncTask).Result;
Ramin
  • 87
  • 2
  • 7