Fix: The synchronization context method Send is now blocking

[saskathex]

The send method of the synchronization context has to block the caller thread in case it is not the main thread.

[tig]

Hi. Thanks for this.

Can you please explain what issue this fixes and how it fixes it?

[saskathex]

Sure. The current "send" method implementation of the synchronization context is in fact equal to the "post" method. It does enqueue/dispatch the action in the idle handler of the main loop.
The microsoft documentation states that the send invokes the action synchronuously. https://docs.microsoft.com/en-us/dotnet/api/system.threading.synchronizationcontext.send?view=net-6.0

The changes: The send method waits until the action is executed in the main thread/loop and blocks the caller until done. There is a differentation between a call from the main thread and a "non"-main thread. To differentiate the main thread the main thread id is being stored during the .Init call.

My use/case is the following:

• Use a task to do some long running work. During the runtime of the task user input is required. So I popup dialog is displayed. The current implementation does not block the caller task until user input is done.
• I used the the same concept in other ui frameworks like gtksharp and winforms and it works correctly.

[tznind]

The current implementation does not block the caller task until user input is done.

Is it worth making the blocking optional? I know winforms has Invoke and InvokeAsync where the first blocks and the second does not. If our current implementation is non blocking then changing it to blocking could be a breaking change? So having both as options would be good so that users who don't want blocking are still able to get it?

[BDisp]

I never hit the Send method on a breakpoint in the GUI.cs. The Post method yes I hit in a breakpoint on async/await.
On a web app the synchronization context is not working. @saskathex do you know if there are similar functionality for a web app?

[tig]

The current implementation does not block the caller task until user input is done.

Is it worth making the blocking optional? I know winforms has Invoke and InvokeAsync where the first blocks and the second does not. If our current implementation is non blocking then changing it to blocking could be a breaking change? So having both as options would be good so that users who don't want blocking are still able to get it?

I'm confused. @saskathex pointed out in his response that right now Send works like Post (non-blocking). This PR (I think) fixes Send so that it is blocking, but does not change the Post behavior. Thus, with this PR

• Send is blocking
• Post is non-blocking

...and therefore the blocking/non-blocking behavior IS optional.

Right?

Regardless, there's a merge error now AND a real need for unit tests.

[tig]

The current implementation does not block the caller task until user input is done.

Is it worth making the blocking optional? I know winforms has Invoke and InvokeAsync where the first blocks and the second does not. If our current implementation is non blocking then changing it to blocking could be a breaking change? So having both as options would be good so that users who don't want blocking are still able to get it?

I'm confused. @saskathex pointed out in his response that right now Send works like Post (non-blocking). This PR (I think) fixes Send so that it is blocking, but does not change the Post behavior. Thus, with this PR

• Send is blocking
• Post is non-blocking

...and therefore the blocking/non-blocking behavior IS optional.

Right?

Regardless, there's a merge error now AND a real need for unit tests.

@saskathex - can you please chime in here?

[saskathex]

Imo making the blocking optional is not way to go. The synchronization context Post/Send methods are generic over all ui frameworks and should be used the correct way. The current implementation is incorrect and should be fixed.

[BDisp]

Imo making the blocking optional is not way to go. The synchronization context Post/Send methods are generic over all ui frameworks and should be used the correct way. The current implementation is incorrect and should be fixed.

Thanks for your feedback. I already tried to get the Send method to be called in the Terminal.Gui but without success. Can you provide a working unit test or sample application where the Send method is invoked, please? Thanks.

By the way you have to resolve the conflicts for this been merged.

[tznind]

Imo making the blocking optional is not way to go. The synchronization context Post/Send methods are generic over all ui frameworks and should be used the correct way.

Sorry for confusing things, I think I was confusing Post/Send with MainLoop.Invoke since that's what I've had the most experience with.

[saskathex]

I will try to add some unit tests.

[BDisp]

I will try to add some unit tests.

Thank you for the unit tests, great. Can you also provide a unit test for the CreateCopy method, please?
I confirm this never blocks the code execution. In practice the d (state) method will always executed normally by the else statement because the Thread.CurrentThread.ManagedThreadId rarely will be equal to _mainThreadId and the wasExecuted field will be true after the Mainloop adds the idle. If Thread.CurrentThread.ManagedThreadId == _mainThreadId runs the function and the called method, normally a task, decide what to do next.
Don't forget to press Ctrl+K+D to reformat the text if you are using VS2022 or the equivalent command if you are using VSCode.
Thanks again for this.

[BDisp]

Thanks @saskathex.

[tig]

Really great work. Thank you!

[tig]

Is this PR good to go now? Can I merge it?

[BDisp]

Is this PR good to go now? Can I merge it?

For me yes.