CODEWITHSUNDEEP
c#asp.netasp.net-mvcasync-await
emery.noel
emery.noel

Reputation: 1185

Why doesn't this call to .Result provoke a deadlock?

I'm still trying to wrap my head around when I can use .Result, and when I can't. I have been trying to avoid it entirely, by using async "all the way down".

While doing some work in our ASP.NET web app (NOT core), I came across this code. Now, the code works (and has been working for 2 years), so I know it works, I just don't know why it works.

This is fired from a sync controller method. It is a chain of method calls that goes through a few layers until it finally does some HTTP work. I have simplified the code here:

// Class1
public byte[] GetDocument1(string url)
{
    return class2.GetDocument2(url).Result;
}

// Class2
public Task<byte[]> GetDocument2(string url)
{
    return class3.GetDocument3(url)
}

// Class3
public async Task<byte[]> GetDocument3(string url)
{
    var client = GetHttpClient(url);
    var resp = client.GetAsync(url).Result;

    if(resp.StatusCode == HttpStatusCode.OK)
    {
        using(var httpStream = await resp.Content.ReadAsStreamAsync())
        {
            // we are also using
            await httpStream.ReadAsync(...);
        }
    }
}

So as far as I can tell, when this all starts, I'm on the "main ASP sync context" (I start in a controller and eventually get to this code). We aren't using any .ConfigureAwait(false), so I believe we are always returning to this context.

  1. In GetDocument3, why doesn't client.GetAsync(url).Result deadlock?
  2. GetDocument3 is mixing .Result with await stuff. In general, is that a good idea?? Is it OK here because .Result comes before the awaits?
  3. In GetDocument1, why doesn't .Result deadlock?

Upvotes: 0

Views: 393

Answers (1)

George Helyar
George Helyar

Reputation: 5288

client.GetAsync(url).Result is synchronously blocking. resp.Content.ReadAsStreamAsync() is actually not doing any awaiting. The Task is already completed, because HttpCompletionOption.ResponseContentRead is used in GetAsync, so the entire block of code here is synchronous code pretending to be asynchronous.

In general, you should never use .Result or .Wait or Task.Wait* - if you absolutely have to, use GetAwaiter().GetResult(), which doesn't throw exceptions wrapped in AggregateExceptions, for which you may not have a catch, but even that should be avoided like the plague. You're right to use async all the way down.

The deadlock is only a problem in a context that wants to return back to the original thread (e.g. UI or ASP.NET), and that thread is blocked by a synchronous wait. If you always use ConfigureAwait(false) on every await, unless you know you actually need to preserve the context on the continuation (usually only at the top level UI event handler because you need to update something on the UI, or in the top level of the ASP.NET controller) then you should be safe.

Synchronous blocking inside asynchronous code also isn't good, but won't cause the deadlock. Using await with ConfigureAwait(true) (the default), inside a synchronous wait, in a context that wants to return back to a specific thread will cause the deadlock.

Upvotes: 5

Related Questions