[DefApp] Move from Monarch multi instance servers to Peasant single instance servers

src/cascadia/TerminalApp/AppActionHandlers.cpp

@@ -257,12 +257,12 @@ namespace winrt::TerminalApp::implementation
     {
         if (args == nullptr)
         {
-            _OpenNewTab(nullptr);
+            LOG_IF_FAILED(_OpenNewTab(nullptr));
             args.Handled(true);
         }
         else if (const auto& realArgs = args.ActionArgs().try_as<NewTabArgs>())
         {
-            _OpenNewTab(realArgs.TerminalArgs());
+            LOG_IF_FAILED(_OpenNewTab(realArgs.TerminalArgs()));
             args.Handled(true);
         }
     }

src/cascadia/TerminalApp/AppLogic.cpp

@@ -1229,6 +1229,11 @@ namespace winrt::TerminalApp::implementation
             auto actions = winrt::single_threaded_vector<ActionAndArgs>(std::move(appArgs.GetStartupActions()));
 
             _root->ProcessStartupActions(actions, false, cwd);
+
+            if (appArgs.IsHandoffListener())
+            {
+                _root->SetInboundListener(true);
+            }
         }
         // Return the result of parsing with commandline, though it may or may not be used.
         return result;

src/cascadia/TerminalApp/TabManagement.cpp

@@ -56,7 +56,7 @@ namespace winrt::TerminalApp::implementation
     // - existingConnection: An optional connection that is already established to a PTY
     //   for this tab to host instead of creating one.
     //   If not defined, the tab will create the connection.
-    void TerminalPage::_OpenNewTab(const NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection)
+    HRESULT TerminalPage::_OpenNewTab(const NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection)
     try
     {
         const auto profileGuid{ _settings.GetProfileForArgs(newTerminalArgs) };
@@ -89,8 +89,10 @@ namespace winrt::TerminalApp::implementation
             TraceLoggingWideString(schemeName.data(), "SchemeName", "Color scheme set in the settings"),
             TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
             TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance));
+
+        return S_OK;
     }
-    CATCH_LOG();
+    CATCH_RETURN();
 
     // Method Description:
     // - Creates a new tab with the given settings. If the tab bar is not being

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -19,6 +19,8 @@
 #include "RenameWindowRequestedArgs.g.cpp"
 #include "../inc/WindowingBehavior.h"
 
+#include <til/latch.h>
+
 using namespace winrt;
 using namespace winrt::Windows::Foundation::Collections;
 using namespace winrt::Windows::UI::Xaml;
@@ -356,34 +358,12 @@ namespace winrt::TerminalApp::implementation
                 winrt::Microsoft::Terminal::TerminalConnection::ConptyConnection::StartInboundListener();
             }
             // If we failed to start the listener, it will throw.
-            // We should fail fast here or the Terminal will be in a very strange state.
-            // We only start the listener if the Terminal was started with the COM server
-            // `-Embedding` flag and we make no tabs as a result.
-            // Therefore, if the listener cannot start itself up to make that tab with
-            // the inbound connection that caused the COM activation in the first place...
-            // we would be left with an empty terminal frame with no tabs.
-            // Instead, crash out so COM sees the server die and things unwind
-            // without a weird empty frame window.
+            // We don't want to fail fast here because if a peasant has some trouble with
+            // starting the listener, we don't want it to crash and take all its tabs down
+            // with it.
             catch (...)
             {
-                // However, we cannot always fail fast because of MSFT:33501832. Sometimes the COM catalog
-                // tears the state between old and new versions and fails here for that reason.
-                // As we're always becoming an inbound server in the monarch, even when COM didn't strictly
-                // ask us yet...we might just crash always.
-                // Instead... we're going to differentiate. If COM started us... we will fail fast
-                // so it sees the process die and falls back.
-                // If we were just starting normally as a Monarch and opportunistically listening for
-                // inbound connections... then we'll just log the failure and move on assuming
-                // the version state is torn and will fix itself whenever the packaging upgrade
-                // tasks decide to clean up.
-                if (_isEmbeddingInboundListener)
-                {
-                    FAIL_FAST_CAUGHT_EXCEPTION();
-                }
-                else
-                {
-                    LOG_CAUGHT_EXCEPTION();
-                }
+                LOG_CAUGHT_EXCEPTION();
             }
         }
     }
@@ -767,7 +747,7 @@ namespace winrt::TerminalApp::implementation
         }
         else
         {
-            this->_OpenNewTab(newTerminalArgs);
+            LOG_IF_FAILED(this->_OpenNewTab(newTerminalArgs));
         }
     }
 
@@ -2363,13 +2343,38 @@ namespace winrt::TerminalApp::implementation
         return _isAlwaysOnTop;
     }
 
-    void TerminalPage::_OnNewConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection)
+    HRESULT TerminalPage::_OnNewConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection)
     {
-        // TODO: GH 9458 will give us more context so we can try to choose a better profile.
-        _OpenNewTab(nullptr, connection);
+        // We need to be on the UI thread in order for _OpenNewTab to run successfully.
+        // HasThreadAccess will return true if we're currently on a UI thread and false otherwise.
+        // When we're on a COM thread, we'll need to dispatch the calls to the UI thread
+        // and wait on it hence the locking mechanism.
+        if (Dispatcher().HasThreadAccess())
+        {
+            // TODO: GH 9458 will give us more context so we can try to choose a better profile.
+            auto hr = _OpenNewTab(nullptr, connection);
 
-        // Request a summon of this window to the foreground
-        _SummonWindowRequestedHandlers(*this, nullptr);
+            // Request a summon of this window to the foreground
+            _SummonWindowRequestedHandlers(*this, nullptr);
+
+            return hr;
+        }
+        else
+        {
+            til::latch latch{ 1 };
+            HRESULT finalVal = S_OK;
+
+            Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [&]() {
+                finalVal = _OpenNewTab(nullptr, connection);
+
+                _SummonWindowRequestedHandlers(*this, nullptr);
+
+                latch.count_down();
+            });
+
+            latch.wait();
+            return finalVal;
+        }
     }
 
     // Method Description:

src/cascadia/TerminalApp/TerminalPage.h

@@ -188,7 +188,7 @@ namespace winrt::TerminalApp::implementation
 
         void _CreateNewTabFlyout();
         void _OpenNewTabDropdown();
-        void _OpenNewTab(const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection = nullptr);
+        HRESULT _OpenNewTab(const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection = nullptr);
         void _CreateNewTabFromSettings(GUID profileGuid, const Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult& settings, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection existingConnection = nullptr);
         winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection _CreateConnectionFromSettings(GUID profileGuid, Microsoft::Terminal::Settings::Model::TerminalSettings settings);
 
@@ -336,7 +336,7 @@ namespace winrt::TerminalApp::implementation
         winrt::Microsoft::Terminal::Settings::Model::Command _lastPreviewedCommand{ nullptr };
         winrt::Microsoft::Terminal::Settings::Model::TerminalSettings _originalSettings{ nullptr };
 
-        void _OnNewConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection);
+        HRESULT _OnNewConnection(winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection connection);
         void _HandleToggleInboundPty(const IInspectable& sender, const Microsoft::Terminal::Settings::Model::ActionEventArgs& args);
 
         void _WindowRenamerActionClick(const IInspectable& sender, const IInspectable& eventArgs);

src/cascadia/TerminalConnection/CTerminalHandoff.cpp

@@ -11,6 +11,8 @@ using namespace Microsoft::WRL;
 static NewHandoffFunction _pfnHandoff = nullptr;
 // The registration ID of the class object for clean up later
 static DWORD g_cTerminalHandoffRegistration = 0;
+// Mutex so we only do start/stop/establish one at a time.
+static std::shared_mutex _mtx;
 
 // Routine Description:
 // - Starts listening for TerminalHandoff requests by registering
@@ -19,9 +21,11 @@ static DWORD g_cTerminalHandoffRegistration = 0;
 // - pfnHandoff - Function to callback when a handoff is received
 // Return Value:
 // - S_OK, E_NOT_VALID_STATE (start called when already started) or relevant COM registration error.
-HRESULT CTerminalHandoff::s_StartListening(NewHandoffFunction pfnHandoff) noexcept
+HRESULT CTerminalHandoff::s_StartListening(NewHandoffFunction pfnHandoff)
 try
 {
+    std::unique_lock lock{ _mtx };
+
     RETURN_HR_IF(E_NOT_VALID_STATE, _pfnHandoff != nullptr);
 
     const auto classFactory = Make<SimpleClassFactory<CTerminalHandoff>>();
@@ -31,7 +35,7 @@ try
     ComPtr<IUnknown> unk;
     RETURN_IF_FAILED(classFactory.As(&unk));
 
-    RETURN_IF_FAILED(CoRegisterClassObject(__uuidof(CTerminalHandoff), unk.Get(), CLSCTX_LOCAL_SERVER, REGCLS_MULTIPLEUSE, &g_cTerminalHandoffRegistration));
+    RETURN_IF_FAILED(CoRegisterClassObject(__uuidof(CTerminalHandoff), unk.Get(), CLSCTX_LOCAL_SERVER, REGCLS_SINGLEUSE, &g_cTerminalHandoffRegistration));
 
     _pfnHandoff = pfnHandoff;
 
@@ -46,8 +50,10 @@ CATCH_RETURN()
 // - <none>
 // Return Value:
 // - S_OK, E_NOT_VALID_STATE (stop called when not started), or relevant COM class revoke error
-HRESULT CTerminalHandoff::s_StopListening() noexcept
+HRESULT CTerminalHandoff::s_StopListening()
 {
+    std::unique_lock lock{ _mtx };
+
     RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _pfnHandoff);
 
     _pfnHandoff = nullptr;
@@ -91,10 +97,19 @@ static HRESULT _duplicateHandle(const HANDLE in, HANDLE& out) noexcept
 // - E_NOT_VALID_STATE if a event handler is not registered before calling. `::DuplicateHandle`
 //   error codes if we cannot manage to make our own copy of handles to retain. Or S_OK/error
 //   from the registered handler event function.
-HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client) noexcept
+HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client)
 {
+    // Stash a local copy of _pfnHandoff before we stop listening.
+    auto localPfnHandoff = _pfnHandoff;
+
+    // Because we are REGCLS_SINGLEUSE... we need to `CoRevokeClassObject` after we handle this ONE call.
+    // COM does not automatically clean that up for us. We must do it.
+    s_StopListening();
+
+    std::unique_lock lock{ _mtx };
+
     // Report an error if no one registered a handoff function before calling this.
-    RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _pfnHandoff);
+    RETURN_HR_IF_NULL(E_NOT_VALID_STATE, localPfnHandoff);
 
     // Duplicate the handles from what we received.
     // The contract with COM specifies that any HANDLEs we receive from the caller belong
@@ -108,5 +123,5 @@ HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE sign
     RETURN_IF_FAILED(_duplicateHandle(client, client));
 
     // Call registered handler from when we started listening.
-    return _pfnHandoff(in, out, signal, ref, server, client);
+    return localPfnHandoff(in, out, signal, ref, server, client);
 }

src/cascadia/TerminalConnection/CTerminalHandoff.h

@@ -37,12 +37,12 @@ struct __declspec(uuid(__CLSID_CTerminalHandoff))
                                      HANDLE signal,
                                      HANDLE ref,
                                      HANDLE server,
-                                     HANDLE client) noexcept override;
+                                     HANDLE client) override;
 
 #pragma endregion
 
-    static HRESULT s_StartListening(NewHandoffFunction pfnHandoff) noexcept;
-    static HRESULT s_StopListening() noexcept;
+    static HRESULT s_StartListening(NewHandoffFunction pfnHandoff);
+    static HRESULT s_StopListening();
 };
 
 // Disable warnings from the CoCreatableClass macro as the value it provides for

src/cascadia/WindowsTerminal/AppHost.cpp

@@ -656,9 +656,6 @@ void AppHost::_BecomeMonarch(const winrt::Windows::Foundation::IInspectable& /*s
                              const winrt::Windows::Foundation::IInspectable& /*args*/)
 {
     _setupGlobalHotkeys();
-
-    // The monarch is just going to be THE listener for inbound connections.
-    _listenForInboundConnections();
 }
 
 void AppHost::_listenForInboundConnections()