19

I have a library that exposes synchronous and asynchronous versions of a method, but under the hood, they both have to call an async method. I can't control that async method (it uses async/await and does not use ConfigureAwait(false)), nor can I replace it.

The code executes in the context of an ASP .NET request, so to avoid deadlocks, here's what I've done:

var capturedContext = SynchronizationContext.Current;
try
{
    // Wipe the sync context, so that the bad library code won't find it
    // That way, we avoid the deadlock
    SynchronizationContext.SetSynchronizationContext(null);

    // Call the async method and wait for the result
    var task = MyMethodAsync();
    task.Wait();

    // Return the result
    return task.Result;
}
finally
{
    // Restore the sync context
    SynchronizationContext.SetSynchronizationContext(capturedContext);
}

Does this produce the same effect as if MyMethodAsync had used ConfigureAwait(false) on all of its await's? Are there some other problems with this approach that I'm overlooking?

(MyMethodAsync is completely unaware that it's being run in an ASP .NET context, it doesn't do any calls to HttpContext.Current or anything like that. It just does some async SQL calls, and the writer didn't put ConfigureAwait(false) on any of them)

2
  • Alternative to null context is to use a single threaded context by using AsyncPump.cs by Stephen Toub: blogs.msdn.microsoft.com/pfxteam/2012/02/02/… AsyncPump will run all continuations in original thread. Setting null context I think is the opposite: continuations will always run in a worker thread. So yes, I think null context is the same as ConfigureAwait(false) everywhere. Both solutions prevents deadlocks.
    – osexpert
    Commented Jan 1, 2019 at 19:51
  • 1
    I use a helper class in my MIT licensed open source project: github.com/jjxtra/ExchangeSharp/blob/master/ExchangeSharp/…. Simply call await new SynchronizationContextRemover(); as the first line of the async method that does not need to await back to the original synchronization context, great for API or other class libraries.
    – jjxtra
    Commented Aug 30, 2019 at 15:22

3 Answers 3

6

Provided you wrap this technique in a suitably named static function, I think your suggest is significantly better than Task.Run, even if still the lesser of two evils.

Task.Run has a number of issues:

  • It is not clear why you are using it, you want to start a new task on a web server? This will be deleted by new developers fast if there are no comments. And then boom, difficult to diagnose production issues (deadlocks).
  • It starts on a new thread pool thread when it doesn't need to until it reaches its first await completed continuation.
  • It makes you block synchronously for the entire Task returning function, when from your description of the problem, the blocking is actually just part of the overall task. What is being encouraged here is longer blocking over async code, this is certainly not what you want.
  • If you use it multiple levels, you are multiplying the problem (with SetSynchronizationContext there's no harm in doing it more than once).
  • If it turns out that there was no blocking / deadlock where you thought there was, or it had been fixed, Task.Run now is introducing blocking over async, whereas SetSynchronizationContext will not cost you anything, in addition to the optimizations it makes by not resuming on the context constantly.

I also understand there is hesitance to make any recommendation given blocking on async code should be avoided at all costs, however you have made it clear you are aware of this and this is to fix a known case of this outside of your immediate control. I think the dogmatic attitude towards this topic is damaging to the .NET ecosystem.

1
  • 1
    I appreciate your answer focussing on the technical merits of the solution :) And I agree with you that the dogmatic attitude in the .NET ecosystem causes more harm than good. I forget where I first heard this, but the quote goes, "There isn't 'clean' code and 'dirty' code. There is code that works and code that doesn't." I still use the terms clean/dirty, but there's something to be said for focussing on function of the code, rather than form.
    – Ryan
    Commented May 1, 2017 at 20:16
6

I have a library that exposes synchronous and asynchronous versions of a method, but under the hood, they both have to call an async method.

The library is wrong to expose a synchronous version. Just pretend the synchronous API doesn't exist.

so to avoid deadlocks

There shouldn't be any problems with deadlocks if you call an asynchronous method that uses async/await. If it doesn't use ConfigureAwait(false), then it's not as efficient as it could be, that's all. Deadlocks due to ConfigureAwait(false) only apply when you're trying to do sync-over-async (i.e., if you're calling the synchronous APIs from that library).

So, the easiest and simplest solution is to just ignore the synchronous APIs, which are incorrectly designed anyway:

return await MyMethodAsync();
6
  • 4
    Even Stephen Toub concedes that sometimes you 'really do need "sync over async.'" I simply can't mandate that all other code (especially legacy code) be async; I have to provide a synchronous version. It's worth mentioning that the async function call only happens in certain cases. The bulk of the function really is truly synchronous; I just have this one corner case to deal with.
    – Ryan
    Commented Aug 2, 2014 at 15:04
  • 1
    In short, I promise that I really am trying to be a good C# dev and follow the async/await best practices. But, in the very rare circumstance where you simply have to bend the rules, is my SynchronizationContext trick a decent alternative to Stephen Toub's "Offload to Another Thread" suggestion?
    – Ryan
    Commented Aug 2, 2014 at 15:06
  • 3
    The library you're using is what is requiring that the code be made async; my first question would be why isn't the library still legacy? But if you insist on sync-over-async (which will impact scalability on ASP.NET), then either Task.Run(...).Result or temporarily replacing SynchronizationContext (which I would do in a using rather than finally) would work. I would lean towards Task.Run for code clarity even though it is slightly less efficient. Commented Aug 2, 2014 at 15:14
  • 1
    This is just a normal, run-of-the-mill mess that you encounter in the real world. The library underneath doesn't have synchronous versions because "we don't have the resources to maintain two copies of every data access function." This code is part of a three-tier caching infrastructure, and the bottom tier that does the SQL calls is the one that doesn't have a synchronous API. But we need to leverage it in some legacy code.
    – Ryan
    Commented Aug 2, 2014 at 15:31
  • @StephenCleary What about situations where you are implementing a interface that has no async version. For example Microsoft.Extensions.Options.IConfigureOptions<T>, If I need to make a call in to a 3rd party library that exposes only a asynchronous method but does not reliably use ConfigureAwait(false). Because I am forced to do a synchronous wait, is using SetSynchronizationContext(null); the best of my available options to call that method without the risk of deadlock? I ask you because I trust your opinion on matters async. Commented Mar 30, 2018 at 17:14
2

Setting the SynchronizationContext to null seems hacky for me. Instead you can really delegate the work to threadpool. Use Task.Run..

var result = Task.Run(() => MyMethodAsync()).Result;

or

var result = Task.Run(async () => await MyMethodAsync()).Result;

This avoids the deadlock and eliminates the hacky code as well.

13
  • Why use an extra threadpool thread? this is a specially dangerous in a ASP.NET environment. how is this not hacky as well? Commented Aug 2, 2014 at 13:16
  • 1
    @usr Your comment is relevant only when SynchronizationContext is set to null. In my answer I don't recommend doing it. Am I missing something ? Commented Aug 2, 2014 at 13:35
  • 1
    When i use an async api, i never assume someone is using a new thread behind the scenes, may it be a threadpool thread or not. In an ASP.NET context, if you don't register that worker thread using HostEnvironment.QueueBackgroundWorkItem, IIS may decide to recycle your app without you ever knowing about it, causing unexpected behavior. I would definitely not want this being done by some 3rd party library i need to use. Commented Aug 2, 2014 at 13:38
  • 1
    @SriramSakthivel no, I agree with you. This works. The 2nd code snippet of yours is not useful, though, because the additional task created there does nothing. It doesn't free any thread.
    – usr
    Commented Aug 2, 2014 at 13:39
  • 2
    Can you guys explain why you think setting the sync context to null is hacky? How is it any different than using ConfigureAwait(false) ?
    – Ryan
    Commented Aug 2, 2014 at 13:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.