dotnet / vblang

The home for design of the Visual Basic .NET programming language and runtime library.
288 stars 66 forks source link

Proposal: Improving async support for better cooperation between sync and async functions #426

Open vbcodec opened 8 years ago

vbcodec commented 8 years ago

Async support is great feature, but there is well known problem with mixing sync and async functions. Handling async procedures form sync procedures is very unpleasant and dangerous (deadlocks), while switching most sync code to async variant is equally problematic and non-practical. To enable better cooperation between sync and async procedures, there is need to create universal functions, that can be used with sync and async calls.

Dim data() As Byte
Public Function ReadFromDisk () As Boolean
    data = Await GetFile() ' GetFile return Taks(Of Byte())
    Return True
End Function

Because this function use await, then is implemented similar to current async functions: creates internal task with state machine, (IAsyncStateMachine). Additionally, such function is aware of external service. If such service exist, then function register their internal task to that service and return nothing. If service do not exist, then executes internal task synchronously to the end, and return result from finished task.

With updated model, service is set for calls with Await keyword, but only for these functions, that can register their internal task. After acquiring task from called universal function, global service is removed. This is alternative way to get task from called function, to current approach where task is grabbed via direct return.

Global service is stack-based, where every thread have separated stack.

Examples how this function and caller work.

Call from synchronous function:

Public Function LoadData () As Boolean
    ReadFromDisk()
    Return True
End Function

LoadData is synchronous, so service is not set, and ReadFromDisk execute their task internally and return result to the caller.

Call from universal function in synchronous way. LoadData is called from other synchronous function:

Public Function LoadData () As Boolean
    Await ReadFromDisk()
    Return True
End Function

Inside this function, Await sets global service. Called function (ReadFromDisk) detect this service, and register internal task to that service. After registering, called function returns Nothing. With acquired task function (LoadData) do not return their own task to the synchronous caller, but wait when acquired task is completed, then executes their own continuation (return True).

If this function (LoadData) is called from async caller, then register their own task to global service, register itself to grabbed task (really awaiter) and return Nothing..

Call from async function:

Public Async Function LoadData() As Task(Of Boolean)
    Await ReadFromDisk()
    Return True
End Function

As above this function grab task from called function via global service, but register itself to awaiter, and return their own task - all just like current behaviour for async functions (except estabilishing service to acuire task)

For cross assembly compatibility, universal function can be marked with attribute (like SyncUniversalAttribute), so compilter will known if called function from library, will register internal task to service or not.

HaloFour commented 8 years ago

then executes internal task synchronously to the end, and return result from finished task.

This is where the vector for deadlocks lies. You can never assume what the async method you called will be doing internally or whether or not it requires the thread that you would be blocking in order to continue. Simply put, it's never safe to mix async and sync, and the language should certainly not encourage developers to jump headfirst into that pit of failure.

vbcodec commented 8 years ago

@HaloFour You are repeating opinions of inexperienced developers, who don't really know how async works, and use 'wishful thinking approach', that result in deadlock. With some knowledge and ideas, there is quick and easy way, to waiting for async function without problems. There is need just to coordinate work of two threads, so deadlock can be prevented. Best place to do it is SynchronizationContext, that was created for such needs and is using by async functions, to give devs possibility to alter default behaviour. The idea is that original thread must be parked, and wiating when other thread want to use it for execute continuation. After executing all continuations, original thread is unparked and may continue sync function., Here is code how to do it. Create new WinForm app, and copy code to Form1.vb

Imports System.Threading
Imports System.Runtime.CompilerServices

Public Class Form1

    Private Sub Form1_Load(sender As Object, e As EventArgs) Handles MyBase.Load
        Dim sc As New SC
        sc.WaitForTask(asf)
    End Sub

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

    Public Async Function asf2() 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
            SynchronizationContext.SetSynchronizationContext(OldContext)
            Exit Sub
        End If

        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

        SynchronizationContext.SetSynchronizationContext(OldContext)
    End Sub

End Class
HaloFour commented 8 years ago

@vbcodec

No, I'm repeating the position that Microsoft has published repeatedly on the subject. Even if it's technically possible to avoid the deadlocks, it goes against the grain of the philosophy of async.

You've demonstrated that your "problem" can be solved entirely through a library and the impact to the consumer is a one-liner. There's no reason to make that a language feature.

vbcodec commented 8 years ago

@HaloFour

You are all wrong again

Microsoft never stated that this is generally impossible. They maybe suggested that it is impossible with default behaviour, when devs do not want to enhance this pattern in any way. They clearly state that async pattern was created to be expandable, and strongly suggest to do it, if there are needs: https://blogs.msdn.microsoft.com/pfxteam/2012/06/15/executioncontext-vs-synchronizationcontext/ https://blogs.msdn.microsoft.com/lucian/2012/12/11/how-to-write-a-custom-awaiter/ and many others. Despite this they want to further reduce remaining fixed behaviour by adding upcoming ValueTask. This is also 'against the grain of the philosophy of async.' ? Nonsense

My proposal is quite difefrent than what you state. I want to make function that perform both synchronously or asynchronously, depending on context where they are called. Provided prototyped context is small part of machinery that enable such behaviour. Read proposal again, until you have doubts..

HaloFour commented 8 years ago

Microsoft never stated that this is generally impossible.

No, they stated that it's a bad idea. Being possible doesn't make it a good idea.

They clearly state that async pattern was created to be expandable, and strongly suggest to do it, if there are needs

To enable awaiting scenarios on things that aren't Tasks. Not to block threads. Even SynchronizationContext isn't designed to do that, it's designed to marshal calls back to an appropriate thread for UI concerns and to manage thread-local state.

My proposal is quite difefrent than what you state. I want to make function that perform both synchronously or asynchronously, depending on context where they are called.

Which is exactly what's wrong with it. There's no way that the compiler or framework could do that in a clean manner. Async functions and sync functions are simply written differently (and they absolutely should be written differently) and you can't make such assumptions as to what the async methods called further may or may not do. The compiler should absolutely not enable calling an async function easily from a sync function.

vbcodec commented 8 years ago

@HaloFour

Dude, you still persistently refuse to accept reality, simple logic, and writing irrelevant claims.

No, they stated that it's a bad idea

Really ? Can you provide links ? In fact they made opposite suggestions, and advertising to use ConfigureAwait(false) to waiting for async methods without creating deadlock. Such trick may be problematic, if UI controls need to be changed. https://channel9.msdn.com/Series/Three-Essential-Tips-for-Async/Async-library-methods-should-consider-using-Task-ConfigureAwait-false-

Not to block threads.

What blocking ? Thanks to duality nature of universal functions, they do not block if are called from async functions. And when called from sync functions, they enable safe calls to async functions and waiting when they finish, without convoluted downsides like changing threads or deadlocks,

SynchronizationContext isn't designed to do that

This is generalized infrastructure, for any model that synchronize thread., not only for UI / non-UI switch.

Async functions and sync functions are simply written differently (and they absolutely should be written differently)

I do not want to alter sync or async functions. because they must perform like they perform currently. My proposal is about adding third type of function, that enable freely mixing sync and async functions without running into serious problems, Universal functions act as smooth bridge between these two different worlds (sync and async).

Please, do not comment further my proposals, as you are heavily inclined towards 'no way' on almost every proposals (my and other users), while providing unsupported claims and fears as explanations. Try to be supportive for this site, and create your own significant proposals,, and convince others to them.

HaloFour commented 8 years ago

Yet you're the one resorting to insults.

The burden of evidence is on you to demonstrate above and beyond that the compiler should be altered in order to accommodate your request. That includes evidence that the scenario is so common and compelling that even if it can be accomplished via a simple library call (which this can, to a varying degree) that it still deserves all of the time and effort and permanent support that is involved with modifying the compiler.

A "universal" method still can't make any assumptions as to what an async method does internally. It may rely on synchronization mechanisms apart from SynchronizationContext such as Control.Invoke or it may require you to call back in through the SynchronizationContext in order to log or maintain state (something you explicitly short-circuit above).

"Async all the way"

qrli commented 8 years ago

whether the function is async as in the syntax is not the root cause. Any function returning a task-like object is async, and need to be handled in async way.

A sync function making async calls need to remove that async-ness, with options like waiting for it (which is typically bad for performance and may deadlock), fire-and-forget, callback, specialized events, etc., as well as many options for exception handling. It is more of a case for specific need and need to be designed with caution. So, I don't think compiler can make a good choice, nor enforce a single choice.

In the meantime, I do think the async function is not ideal, which cause caller to become async as long as any callee is async. It does make code read much better, but it also introduced async-ness everywhere, which means concurrency issues everywhere, and protection codes against failure cases created by concurrency.