Fix dialogs not working across multiple windows

src/cascadia/TerminalApp/TerminalWindow.cpp

@@ -144,7 +144,6 @@ namespace winrt::TerminalApp::implementation
     {
         // Now that we know we can do XAML, build our page.
         _root = winrt::make_self<TerminalPage>(*_WindowProperties, _manager);
-        _dialog = ContentDialog{};
 
         // Pass in information about the initial state of the window.
         // * If we were supposed to start from serialized "content", do that,
@@ -313,6 +312,14 @@ namespace winrt::TerminalApp::implementation
     {
         return _settings.GlobalSettings().CurrentTheme();
     }
+
+    // WinUI can't show 2 dialogs simultaneously. Yes, really. If you do, you get an exception.
+    // As such, we must dismiss whatever dialog is currently being shown.
+    //
+    // This limit is of course per-thread and not per-window. Yes... really.
+    // The consequence is that we use a static variable to keep track of the shown dialog.
+    static ContentDialog s_activeDialog{ nullptr };
+
     // Method Description:
     // - Show a ContentDialog with buttons to take further action. Uses the
     //   FrameworkElements provided as the title and content of this dialog, and
@@ -328,16 +335,32 @@ namespace winrt::TerminalApp::implementation
     // - an IAsyncOperation with the dialog result
     winrt::Windows::Foundation::IAsyncOperation<ContentDialogResult> TerminalWindow::ShowDialog(winrt::WUX::Controls::ContentDialog dialog)
     {
-        // DON'T release this lock in a wil::scope_exit. The scope_exit will get
-        // called when we await, which is not what we want.
-        std::unique_lock lock{ _dialogLock, std::try_to_lock };
-        if (!lock)
+        // As mentioned on s_activeDialog, dismissing the active dialog is necessary.
+        // We repeat it a few times in case the resume_foreground failed to work,
+        // but I found that one iteration will always be enough in practice.
+        for (int i = 0; i < 3; ++i)
+        {
+            if (!s_activeDialog)
+            {
+                break;
+            }
+
+            s_activeDialog.Hide();
+
+            // Wait for the current dialog to be hidden.
+            co_await wil::resume_foreground(_root->Dispatcher(), CoreDispatcherPriority::Low);
+        }
+
+        // If two sources call ShowDialog() simultaneously, it may happen that both enter the above loop,
+        // but it's crucial that only one of them continues below as only 1 dialog can be shown at a time.
+        // Thankfully, everything runs on the UI thread, so only 1 caller will exit the above loop at a time.
+        // So, if s_activeDialog is still set at this point, we must've lost the race.
+        if (s_activeDialog)
         {
-            // Another dialog is visible.
             co_return ContentDialogResult::None;
         }
 
-        _dialog = dialog;
+        s_activeDialog = dialog;
 
         // IMPORTANT: This is necessary as documented in the ContentDialog MSDN docs.
         // Since we're hosting the dialog in a Xaml island, we need to connect it to the
@@ -367,23 +390,26 @@ namespace winrt::TerminalApp::implementation
             }
         } };
 
-        themingLambda(dialog, nullptr); // if it's already in the tree
-        auto loadedRevoker{ dialog.Loaded(winrt::auto_revoke, themingLambda) }; // if it's not yet in the tree
+        auto result = ContentDialogResult::None;
 
-        // Display the dialog.
-        co_return co_await dialog.ShowAsync(Controls::ContentDialogPlacement::Popup);
+        // Extra scope to drop the revoker before resetting the s_activeDialog to null.
+        {
+            themingLambda(dialog, nullptr); // if it's already in the tree
+            auto loadedRevoker{ dialog.Loaded(winrt::auto_revoke, themingLambda) }; // if it's not yet in the tree
+            result = co_await dialog.ShowAsync(Controls::ContentDialogPlacement::Popup);
+        }
 
-        // After the dialog is dismissed, the dialog lock (held by `lock`) will
-        // be released so another can be shown
+        s_activeDialog = nullptr;
+        co_return result;
     }
 
     // Method Description:
     // - Dismiss the (only) visible ContentDialog
     void TerminalWindow::DismissDialog()
     {
-        if (auto localDialog = std::exchange(_dialog, nullptr))
+        if (s_activeDialog)
         {
-            localDialog.Hide();
+            s_activeDialog.Hide();
         }
     }
 

src/cascadia/TerminalApp/TerminalWindow.h

@@ -167,8 +167,6 @@ namespace winrt::TerminalApp::implementation
         // ALSO: If you add any UIElements as roots here, make sure they're
         // updated in _ApplyTheme. The root currently is _root.
         winrt::com_ptr<TerminalPage> _root{ nullptr };
-        winrt::Windows::UI::Xaml::Controls::ContentDialog _dialog{ nullptr };
-        std::shared_mutex _dialogLock;
 
         wil::com_ptr<CommandlineArgs> _appArgs{ nullptr };
         bool _hasCommandLineArguments{ false };