Can no longer nest Application.MainLoop.Invoke (since 1.8.2)

[tznind]

Describe the bug
This is a behavior change in 1.8.2 and not present in 1.7.2

If a view is launched via Invoke e.g.

btnLaunch.Clicked += ()=>{
    Application.MainLoop.Invoke(()=>Application.Run(startWindow));
    };

Then any calls to Invoke that are triggered from the startWindow are blocking and never finish until the window is closed.

While the issue is easiest to demonstrate by launching a window via Invoke I think the issue manifests in other ways too. For example I have a window where callbacks cannot be registered in 1.8.2 (blocks until window is closed) - see HicServices/RDMP#1448 .

I have looked hard and can't see any problematic Invokes in my code but I think it is definitely related to the new locking behavior

UPDATE: Ok I know why my code is getting this deadlock, it is because of the use of ContextMenu. If you update the demo code to launch the test window via a menu you get same problem:

var menu = new ContextMenu();
menu.MenuItems = new MenuBarItem(new []{new MenuItem("Launch",null,()=>Application.Run(startWindow))});

    btnLaunch.Clicked += ()=>{
        menu.Show();
    };

When running on 1.8.2 the 'Click Me' button gets stuck at 'Cancel' and its callback to increment the counter never happens. So total only increases by 1 when you press it instead of 4. In 1.7.2 it works correctly

To Reproduce
I have created a minimum repro:

using Terminal.Gui;

class Program
{
    static int total = 0;
    static Button btn;
    public static void Main()
    {
        Application.Init();

        var startWindow = new Window{
            Modal = true
        };

        btn = new Button{
            Text = "Click Me"
        };

        btn.Clicked += RunAsyncTest;

        var totalbtn = new Button(){
            X = Pos.Right(btn),
            Text = "total"
        };

        totalbtn.Clicked += ()=>{
          MessageBox.Query("Count",$"Count is {total}","Ok");
        };


        startWindow.Add(btn);
        startWindow.Add(totalbtn);

        var splash = new View();
        var btnLaunch = new Button("Open Window");
        
        // Goes fine
        /*btnLaunch.Clicked += ()=>{
            Application.Run(startWindow);
            };*/
        // Goes badly
        btnLaunch.Clicked += ()=>{
            Application.MainLoop.Invoke(()=>Application.Run(startWindow));
            };

        splash.Add(btnLaunch);

        Application.Top.Add(btnLaunch);

        Application.Run();

        Application.Shutdown();

    }

    private static void RunAsyncTest()
    {
        btn.Text = "Cancel";
        Interlocked.Increment(ref total);
        btn.SetNeedsDisplay();
        
        var task = Task.Run(()=>
        {
            try
            {
                RunSql();
            }
            finally{
                SetReadyToRun();
            }
        }).ContinueWith((s,e)=> {
            
            Interlocked.Increment(ref total);

            },TaskScheduler.FromCurrentSynchronizationContext());
    }

    private static void RunSql()
    {        
        Thread.Sleep(100);
        Application.MainLoop.Invoke(() => { 
            Interlocked.Increment(ref total);
            btn.Text = "Pew Pew";
            btn.SetNeedsDisplay();
        });
    }

    private static void SetReadyToRun()
    {
        Application.MainLoop.Invoke(()=>{
            btn.Text = "Click Me";
            Interlocked.Increment(ref total);
            btn.SetNeedsDisplay();
        });
    }
}

For csproj use:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Terminal.Gui" Version="1.8.2" />
  </ItemGroup>

</Project>

Run program then change back to 1.7.2 package reference

Expected behavior
Not sure, I'm open to discussion

[BDisp]

This is due to the #1994. @tig had alerted about (BREAKING CHANGE). So the question is would if this is the write behavior, avoiding tasks popup on the main thread that uses a modal as true, or should to be reverted to the previous behavior?

[tznind]

I think in terms of user stories we want to start with the following:

I want to be able to launch modal dialogs from Button or ContextMenu click handlers. And for those windows to behave normally (i.e. be able to use their own invokes).

Currently launching a modal Window from Button behaves differently from ContextMenu (The difference being that the resulting dialog cannot do its own invokes). I think because Action gets run through invoke in Menu:

Terminal.Gui/Terminal.Gui/Views/Menu.cs

Lines 571 to 584 in 32e7b63

	public void Run (Action action)
	{
	if (action == null)
	return;
	Application.UngrabMouse ();
	host.CloseAllMenus ();
	Application.Refresh ();
	Application.MainLoop.AddIdle (() => {
	action ();
	return false;
	});
	}

While in Button it is not:

Terminal.Gui/Terminal.Gui/Views/Button.cs

Lines 229 to 235 in 7b93f44

	/// <summary>
	/// Virtual method to invoke the <see cref="Clicked"/> event.
	/// </summary>
	public virtual void OnClicked ()
	{
	Clicked?.Invoke ();
	}

We should understand why they are behaving differently, which one is more correct and how we can adjust the current code to still have the good stable locking stuff added in #1994 but also allow for this user story.

[BDisp]

Sorry, I made confusion because I thought you wanted that the follow should work, but was only to demonstrate the behavior you have mention.
Application.MainLoop.Invoke(()=>Application.Run(startWindow));

The menu run action is the same as the original but since the idle handlers lock was fixed, running a action through the Application.MainLoop.Invoke will block another following calls through the Application.MainLoop.Invoke because the running action block the idle handlers. If you wish I can submit a PR to change the Menu action calls or if you already have a fix for this I let you fix it.

[tznind]

Sorry, I made confusion because I thought you wanted that the follow should work, but was only to demonstrate the behavior you have mention. Application.MainLoop.Invoke(()=>Application.Run(startWindow));

Yeah my post was a bit muddled, sorry about that. I created Issue when I had a repro but before I had traced it down to ContextMenu callback being the responsible.

Good fix though! Thanks