Use Task.Run() in synchronous method to avoid deadlock waiting on async method?
Asked Answered
V

5

60

UPDATE The purpose of this question is to get a simple answer about Task.Run() and deadlocking. I very much understand the theoretical reasoning for not mixing async and sync, and I take them to heart. I'm not above learning new things from others; I seek to do that whenever I can. There's just times when all a guy needs is a technical answer...

I have a Dispose() method that needs to call an async method. Since 95% of my code is async, refactoring isn't the best choice. Having an IAsyncDisposable (among other features) that's supported by the framework would be ideal, but we're not there yet. So in the mean time, I need to find a reliable way to call async methods from a synchronous method without deadlocking.

I'd prefer not to use ConfigureAwait(false) because that leaves the responsibility scattered all throughout my code for the callee to behave a certain way just in case the caller is synchronous. I'd prefer to do something in the synchronous method since it's the deviant bugger.

After reading Stephen Cleary's comment in another question that Task.Run() always schedules on the thread pool even async methods, it made me think.

In .NET 4.5 in ASP.NET or any other synchronization context that schedules tasks to the current thread / same thread, if I have an asynchronous method:

private async Task MyAsyncMethod()
{
    ...
}

And I want to call it from a synchronous method, can I just use Task.Run() with Wait() to avoid deadlocks since it queues the async method the the thread pool?

private void MySynchronousMethodLikeDisposeForExample()
{
    // MyAsyncMethod will get queued to the thread pool 
    // so it shouldn't deadlock with the Wait() ??
    Task.Run((Func<Task>)MyAsyncMethod).Wait();
}
Vain answered 3/2, 2015 at 18:18 Comment(10)
if MyAsyncMethod() is marshaling back to the calling context, presumably it actually needs to execute in that context or it won't work. Fundimentally you should avoid this problem entirely. Either make the program synchronous, or make it asynchronous. Doing half and half is only going to cause problems.Lewse
You clean up resources asynchronously? That seems a bit suspicious. Clean up shouldn't be something that I would expect to require asynchronous work.Lewse
@Servy, my Dispose() method calls stuff that's used elsewhere. I like to reuse code instead of writing it twice.Vain
@Servy, and yes, I may eventually stop using the IDisposable interface because it doesn't support async, but we're in a transition with .NET where async is there but not fully supported in the language, so we're going to run into this problem over and over again until the framework and language features grow a bit more.Vain
I'm not saying you should be doing it twice. I'm saying that you shouldn't be doing IO, or work that is offloaded to a another thread, or other types of operations that would have reason to be asynchronous in the first place (or any operations composing those types of operations). The idea of doing those types of expensive operations, even synchronously, in a Dispose method is at least concerning.Lewse
@Servy, I'm mainly looking for a simple answer as to whether or not my proposed idea of using Task.Run() to avoid deadlocks will work reliably. Like I said 95% of my code is async. There's still places in the .NET that don't support async. I try to eliminate the synchronous code wherever I can and when time allows, but ultimately I end up needing to call asynchronous code from synchronous code.Vain
And ultimately every single possible approach you can use to solve that problem is going to cause more problems (at least in certain circumstances). The only way to really solve the problem without causing more, is to avoid it from happening in the first place.Lewse
@Vain - why do you rule out manually creating "IDisposeAsync"? Since using/IDispose is just tiny amount of syntactic sugar asynchronous creating helper function like Task UsingAsync(Func<T> createResource, Action<T> useResource) where T:IDisposeAsync should produce comparable code...Hundredfold
@AlexeiLevenkov I have no problem with writing IDisposeAsync. I've been developing frameworks for over 20 years. I do like to do things correctly. I push the envelope when it comes to spending time refactoring. But there comes a point in most real life large scale projects where you just have to stop refactoring and get things done... Are you guys trying to tell me you have large scale applications that use async and you never run into a need to call something async from synchronous code?Vain
Yes, and the single place we had to call async method synchronously the way you show caused unproportional number of issues. If you are building framework you have reasonable control over what being used/passed around... But with regular non-reusable code effort to always pass context around does not necessary pays off - so as soon as you have method that does not follow default rules (run with valid UI/ASP.Net context) you get all sorts of random issues - wrong culture, missing HttpContext.Current, blowing up UI updates...Hundredfold
E
86

It seems you understand the risks involved in your question so I'll skip the lecture.

To answer your actual question: Yes, you can just use Task.Run to offload that work to a ThreadPool thread which doesn't have a SynchronizationContext and so there's no real risk for a deadlock.

However, using another thread just because it has no SC is somewhat of a hack and could be an expensive one since scheduling that work to be done on the ThreadPool has its costs.

A better and clearer solution IMO would be to simply remove the SC for the time being using SynchronizationContext.SetSynchronizationContext and restoring it afterwards. This can easily be encapsulated into an IDisposable so you can use it in a using scope:

public static class NoSynchronizationContextScope
{
    public static Disposable Enter()
    {
        var context = SynchronizationContext.Current;
        SynchronizationContext.SetSynchronizationContext(null);
        return new Disposable(context);
    }

    public struct Disposable : IDisposable
    {
        private readonly SynchronizationContext _synchronizationContext;

        public Disposable(SynchronizationContext synchronizationContext)
        {
            _synchronizationContext = synchronizationContext;
        }

        public void Dispose() =>
            SynchronizationContext.SetSynchronizationContext(_synchronizationContext);
    }
}

Usage:

private void MySynchronousMethodLikeDisposeForExample()
{
    using (NoSynchronizationContextScope.Enter())
    {
        MyAsyncMethod().Wait();
    }
}
Erkan answered 3/2, 2015 at 20:17 Comment(16)
Hmmmm..... I like that. Places where I need to do this are minimal, like Application_End() in an ASP.NET app, and the thread pool overhead probably isn't a significant factor, but this approach definitely is more direct and clean. Your answer just made an aspect of SynchronizationContextclick as well.Vain
Just a thought, if there's no SynchronizationContext, isn't the default context to use the ThreadPool? So the effect would be the same as Run.Task()Vain
@Vain No. In both cases there's no SC, so the code after the first await in MyAsyncMethod would be posted to the thread pool, however the code before that await (called the synchronous part of the async method) would run on the calling thread. In my answer that thread would be the UI thread, and Task.Run uses a thread pool thread. In the case where there's no await at all, or the awaited task completed synchronously my answer would use only the UI thread.Erkan
Task.Run is using a thread pool thread from the start which means it has an extra chuck to schedule. That may be desired in some cases, but it redundant if all you want is to remove the SC.Erkan
OK that makes sense. In my case with a synchronous caller there's no need for it to be on a separate thread, and even if I do find an instance in a synchronous caller where I want to do "old fashioned" start a thread, continue processing, wait for thread, if the async method is written properly, the first await will be at the first optimizable waiting point in the method, so there's no point in scheduling the whole thing on a separate thread from the start. So either way, your method wins over Task.Run()Vain
FYI -- I put your class in my class library and added a couple static helper methods to simplify it into a single line pastebin.com/feHtWPwX, e.g. NoSynchronizationContextScope.RunSynchronously(MyAsyncMethod)Vain
@Vain then make the constructor private.Erkan
Except that I may want to use the NoSynchronizationContextScope in a using block with multiple calls.Vain
A very good reason you may need to do this: ActonResult.Execute. There is no way to make that async, yet it's in the request context. This worked perfectly.Jackanapes
@Erkan there are cases when you must to exit method in the same thread you entered it. Switching, even temporary, synchronization context will cause thread switch as well.Chiba
@DmitryNaumov changing the SC will not switch threads. At least not by itself.Erkan
Please beware that Task.Run does not necessarily spawn a new thread. Payload can actually be executed on the very same thread, so be careful, asynchrony is always there to bite :) #46493323Cyclopean
@EugeneD.Gubenkov Task.Run never spawns new threads. It schedules work on one of the ThreadPool threads. Moreover, if the task is inlined, the thread isn't blocked, it's executing the task it's waiting for so there shouldn't be a deadlock to begin with. In your question the deadlock doesn't come from waiting on a Task.Run task... it comes from that task blocking on some other task (which there's no reason to do).Erkan
@i3arnon, you are right, actually that what I mean by blurry phrase "spawn a new thread" = "actually leading to execution on another thread". Point taken, thanks!Cyclopean
@i3arnon, talking about referenced question, the very fact of inlining, even though it does not "directly" cause deadlock, leads to SyncronizationContext being kept and then internal task waiting causing deadlock (as you fairly pointed out). But the point is that without inlining deadlock should not have happened.Cyclopean
This method might be problematic for scenarios where a custom TaskScheduler is being used. See Issues with NoSynchronizationContextScope and Orleans on the Npgsql repository for lengthy discussion about it.Oddball
C
5

This is my way of avoiding deadlock when I have to call async method synchronously and the thread can be UI thread:

    public static T GetResultSafe<T>(this Task<T> task)
    {
        if (SynchronizationContext.Current == null)
            return task.Result;

        if (task.IsCompleted)
            return task.Result;

        var tcs = new TaskCompletionSource<T>();
        task.ContinueWith(t =>
        {
            var ex = t.Exception;
            if (ex != null)
                tcs.SetException(ex);
            else
                tcs.SetResult(t.Result);
        }, TaskScheduler.Default);

        return tcs.Task.Result;
    }
Chiba answered 2/12, 2016 at 8:35 Comment(1)
This works only for cold tasks (those which are not started). For hot tasks which is already executing in the UI thread, the .Result will still deadlock.Irisirisa
H
3

This code will not deadlock for exactly the reasons you highlighted in the question - code always runs with no synchronization context (since using thread pool) and Wait will simply block the thread till/if method returns.

Hundredfold answered 3/2, 2015 at 19:24 Comment(0)
P
2

If you absolutely must call the async method from an synchronous one, make sure to use ConfigureAwait(false) inside your async method calls to avoid the capturing of the synchronization context.

This should hold but is shaky at best. I would advise to think of refactoring. instead.

Protero answered 3/2, 2015 at 18:21 Comment(2)
I assume this is supposed to be "If you absolutely must call the async method from a synchronous one", and not the current form which says "async method from an asynchronous one"?Diao
@BrentRittenhouse Yup.Protero
U
1

With small custom synchronization context, sync function can wait for completion of async function, without creating deadlock. Original thread is preserved, so sync method use the same thread before and after call to async function. Here is small example for WinForms app.

Imports System.Threading
Imports System.Runtime.CompilerServices

Public Class Form1

    Private Sub Form1_Load(sender As Object, e As EventArgs) Handles MyBase.Load
        SyncMethod()
    End Sub

    ' waiting inside Sync method for finishing async method
    Public Sub SyncMethod()
        Dim sc As New SC
        sc.WaitForTask(AsyncMethod())
        sc.Release()
    End Sub

    Public Async Function AsyncMethod() As Task(Of Boolean)
        Await Task.Delay(1000)
        Return True
    End Function

End Class

Public Class SC
    Inherits SynchronizationContext

    Dim OldContext As SynchronizationContext
    Dim ContextThread As Thread

    Sub New()
        OldContext = SynchronizationContext.Current
        ContextThread = Thread.CurrentThread
        SynchronizationContext.SetSynchronizationContext(Me)
    End Sub

    Dim DataAcquired As New Object
    Dim WorkWaitingCount As Long = 0
    Dim ExtProc As SendOrPostCallback
    Dim ExtProcArg As Object

    <MethodImpl(MethodImplOptions.Synchronized)>
    Public Overrides Sub Post(d As SendOrPostCallback, state As Object)
        Interlocked.Increment(WorkWaitingCount)
        Monitor.Enter(DataAcquired)
        ExtProc = d
        ExtProcArg = state
        AwakeThread()
        Monitor.Wait(DataAcquired)
        Monitor.Exit(DataAcquired)
    End Sub

    Dim ThreadSleep As Long = 0

    Private Sub AwakeThread()
        If Interlocked.Read(ThreadSleep) > 0 Then ContextThread.Resume()
    End Sub

    Public Sub WaitForTask(Tsk As Task)
        Dim aw = Tsk.GetAwaiter

        If aw.IsCompleted Then Exit Sub

        While Interlocked.Read(WorkWaitingCount) > 0 Or aw.IsCompleted = False
            If Interlocked.Read(WorkWaitingCount) = 0 Then
                Interlocked.Increment(ThreadSleep)
                ContextThread.Suspend()
                Interlocked.Decrement(ThreadSleep)
            Else
                Interlocked.Decrement(WorkWaitingCount)
                Monitor.Enter(DataAcquired)
                Dim Proc = ExtProc
                Dim ProcArg = ExtProcArg
                Monitor.Pulse(DataAcquired)
                Monitor.Exit(DataAcquired)
                Proc(ProcArg)
            End If
        End While

    End Sub

     Public Sub Release()
         SynchronizationContext.SetSynchronizationContext(OldContext)
     End Sub

End Class
Unmeasured answered 27/9, 2016 at 13:56 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.