12

I'm calling an async library method with .ConfigureAwait(false). But, I still end up with deadlock. (I'm using it in ASP.NET controller API) But, if I use the same method wrapped into Task.Run() it works fine.

My understanding is, if the libraries method is not using ConfigureAwait internally then adding ConfigureAwait won't solve the problem as in the library call it will result in deadlock (we block on it by using .Result). But, if that's the case why does it work in Task.Run() as it will fail to continue in same context/thread.

This article talks about it. Btw, I have read plenty of articles by Stephen Cleary. But, why Task.Run() works is a mystery.

Code snippet:

// This Create Method results in Deadlock
public async Task<string> Create(MyConfig config)
{
        Document doc = await Client.CreateDocumentAsync(CollectionUri, config).ConfigureAwait(false);
        return doc.Id;
}

// Uses Task.Run() which works properly, why??
public string Create(MyConfig config)
{
    Document doc = Task.Run(() => Client.CreateDocumentAsync(CollectionUri, config)).Result;
    return doc.Id;
}

[HttpPost]
public ActionResult CreateConfig(MyConfig config)
{
     string id = Create(config).Result;
     return Json(id);
}
nilsK
  • 4,323
  • 2
  • 26
  • 40
Krunal Modi
  • 135
  • 1
  • 1
  • 7

3 Answers3

15

I believe Lukazoid is correct. To put it another way...

// This Create Method results in Deadlock
public async Task<string> Create(MyConfig config)
{
  Document doc = await Client.CreateDocumentAsync(CollectionUri, config).ConfigureAwait(false);
  return doc.Id;
}

You can't just stick a ConfigureAwait(false) at one level and have it magically prevent deadlocks. ConfigureAwait(false) can only prevent deadlocks if it is used by every await in the transitive closure of that method and all methods it calls.

In other words, ConfigureAwait(false) needs to be used for every await in Create (which it is), and it also needs to be used for every await in CreateDocumentAsync (which we don't know), and it also needs to be used for every await in every method that CreateDocumentAsync calls, etc.

This is one reason why it is such a brittle "solution" to the deadlock problem.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • 1
    Every async call should be adequately documented on that matter in order to take the right decision. Using `await [async call].ConfigureAwait(false)` or `await Task.Run(async ()=> await [async call]).ConfigureAwait(false)`. – Thanasis Ioannidis Mar 22 '17 at 10:55
  • 1
    @ThanasisIoannidis: That would be great in theory, but in reality a missing `ConfigureAwait(false)` is usually a mistake. So the documentation would just be wrong. – Stephen Cleary Mar 22 '17 at 13:29
  • 1
    can we say that "all the way configreawait(false)" like it is said "all the way await"? – Emil Nov 21 '18 at 16:10
  • 2
    @batmaci: Yes, in the sense that if you're going to use it, it's best to use it all the way. – Stephen Cleary Nov 21 '18 at 17:46
8

In the first example, the implementation of Client.CreateDocumentAsync is deadlocking because it is trying to execute a continuation using the current SynchronizationContext.

When using Task.Run, the delegate will be invoked on a ThreadPool thread, this means there will be no current SynchronizationContext so all continuations will be resumed using a ThreadPool thread. This means it will not deadlock.

Out of interest, why is your CreateConfig method not async? Most recent versions of MVC and WebAPI support asynchronous methods, getting rid of the .Result would be the best solution.

Lukazoid
  • 19,016
  • 3
  • 62
  • 85
  • 1
    Shouldn't use of ConfigureAwait(false) ensure continuation using ThreadPool? and yes, I have made CreateConfig method async. But, wanted to understand the concept better exactly on why Task.Run() doesn't create any problems. – Krunal Modi Apr 15 '16 at 18:45
  • 5
    @KrunalModi `ConfigureAwait(false)` only configures the continuations to use the threadpool, not the invocation of `Client.CreateDocumentAsync` – Lukazoid Apr 15 '16 at 19:50
  • @KrunalModi there is no point using `ConfigureAwait` with an async call that internally has another async call that *is not* used with `ConfigureAwait`. `ConfigureAwait(false)` should be used *almost* anywhere, and especially in libraries. Only the top level calls (those in the UI Synchronization Context) don't need `ConfigureAwait(false)` because the continuation should be executed in the UI Synchronization Context. For libraries that don't use `ConfigureAwait(false)` internally, using `await Task.Run(()=> ...)` or `await Task.Run(()=> ...).ConfigureAwait(false)` is a good workaround. – Thanasis Ioannidis Mar 22 '17 at 10:51
0

Just an observation: i also noticed that doing this will also result in a deadlock

private string Create(Task<Document> task)
{
    var doc = Task.Run(() => task).Result;
    return doc.Id;
}

[HttpPost]
public ActionResult CreateConfig(MyConfig config)
{
     var task = Client.CreateDocumentAsync(CollectionUri, config);
     var id = Create(task).Result;
     return Json(id);
}

So even running things on the thread pool may not be the ultimate solution. It seems an equally import factor to consider is what SynchonizationContext was in effect when the async method's task was created.

Dantte
  • 685
  • 5
  • 10