Repository: microsoft/terminal (language: cpp)

Issue:
Version: 1.20.10572.0 reopens last tabs whenever it feels like it
### Windows Terminal version

Version: 1.20.10572.0

### Windows build number

Version	10.0.22631 Build 22631

### Other Software

_No response_

### Steps to reproduce

Open terminal preview.
Open several tabs.
Restart computer either before or after you close down Terminal.
Wonder if it'll restore windows or it won't.

### Expected Behavior

Do what the option says: reopen tabs from last session.
![fucking-microsoft-again](https://github.com/microsoft/terminal/assets/16505052/ee7a2399-92ab-4300-8ed1-f9b578e2d3c3)


### Actual Behavior

randomly opens last session tabs whenever it feels like it. No option to save my own and restore manually, I'm just living at the mercy of my own (removed expletive to comply with Code of Conduct) computer. It's great.


Discussion / hints:
Please adhere to this repository’s Code of Conduct when you are reporting issues.
Sure thing. Was it "d***" that upset you?
"damn" is not a problem, your post is simply just too sarcastic. For what reason? I'd understand it if we had done something to offend you, but it's just a bug in the application.
(Edit: To be clear, it's not a big deal. We all use sarcasm after all and there's no reason for you to edit it now. It's just that it'd be more productive if you didn't do it again.)

In any case, we're already aware about this issue and still collecting information when and why it happens. It's likely a race condition during shutdown or something similar, which is why it seems "random". My current theory is that it only happens if you have more than one window open.
Got it.

Your current theory matches what I experienced about a year ago (don't remember version) when I curbed my behavior to have more than 1 terminal window open before I close it. So this bug also happens when a single window with multiple tabs is open.

I think I made it happen once or twice when I was trying to reproduce it by killing the process from [Process Hacker](https://processhacker.sourceforge.io/).
If I were to posit a theory: I'm guessing you've got some other CLI process that starts at startup. The OS starts that process up, and it picks Terminal as the default terminal handler, and starts the Terminal. Then, when the Terminal starts, it _also_ starts up all the other windows for itself. 

Now I'm guessing that the reason why sometimes this _doesn't_ happen is along the lines of #17179 - sometimes, the window state _isn't_ getting persisted if you just straight shutdown the PC. 

Fix patch:
diff --git a/src/cascadia/Remoting/Monarch.cpp b/src/cascadia/Remoting/Monarch.cpp
index 63e74b0fae1..b986bfc8630 100644
--- a/src/cascadia/Remoting/Monarch.cpp
+++ b/src/cascadia/Remoting/Monarch.cpp
@@ -96,7 +96,6 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
 
             peasant.ShowNotificationIconRequested([this](auto&&, auto&&) { ShowNotificationIconRequested.raise(*this, nullptr); });
             peasant.HideNotificationIconRequested([this](auto&&, auto&&) { HideNotificationIconRequested.raise(*this, nullptr); });
-            peasant.QuitAllRequested({ this, &Monarch::_handleQuitAll });
 
             {
                 std::unique_lock lock{ _peasantsMutex };
@@ -134,7 +133,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
     // - <none> used
     // Return Value:
     // - <none>
-    void Monarch::_handleQuitAll(const winrt::Windows::Foundation::IInspectable& /*sender*/, const winrt::Windows::Foundation::IInspectable& /*args*/)
+    void Monarch::QuitAll()
     {
         if (_quitting.exchange(true, std::memory_order_relaxed))
         {
@@ -166,12 +165,6 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
             {
                 peasantSearch->second.Quit();
             }
-            else
-            {
-                // Somehow we don't have our own peasant, this should never happen.
-                // We are trying to quit anyways so just fail here.
-                assert(peasantSearch != _peasants.end());
-            }
         }
     }
 
diff --git a/src/cascadia/Remoting/Monarch.h b/src/cascadia/Remoting/Monarch.h
index 299932d9610..4cc74d0c5bd 100644
--- a/src/cascadia/Remoting/Monarch.h
+++ b/src/cascadia/Remoting/Monarch.h
@@ -86,6 +86,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
 
         uint64_t AddPeasant(winrt::Microsoft::Terminal::Remoting::IPeasant peasant);
         void SignalClose(const uint64_t peasantId);
+        void QuitAll();
 
         uint64_t GetNumberOfPeasants();
 
diff --git a/src/cascadia/Remoting/Monarch.idl b/src/cascadia/Remoting/Monarch.idl
index c81837e96b5..ba00cf47cfe 100644
--- a/src/cascadia/Remoting/Monarch.idl
+++ b/src/cascadia/Remoting/Monarch.idl
@@ -63,6 +63,7 @@ namespace Microsoft.Terminal.Remoting
         void HandleActivatePeasant(WindowActivatedArgs args);
         void SummonWindow(SummonWindowSelectionArgs args);
         void SignalClose(UInt64 peasantId);
+        void QuitAll();
 
         void SummonAllWindows();
         Boolean DoesQuakeWindowExist();
diff --git a/src/cascadia/Remoting/Peasant.cpp b/src/cascadia/Remoting/Peasant.cpp
index 6378854d199..e09f5ac67ee 100644
--- a/src/cascadia/Remoting/Peasant.cpp
+++ b/src/cascadia/Remoting/Peasant.cpp
@@ -260,22 +260,6 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
                           TraceLoggingKeyword(TIL_KEYWORD_TRACE));
     }
 
-    void Peasant::RequestQuitAll()
-    {
-        try
-        {
-            QuitAllRequested.raise(*this, nullptr);
-        }
-        catch (...)
-        {
-            LOG_CAUGHT_EXCEPTION();
-        }
-        TraceLoggingWrite(g_hRemotingProvider,
-                          "Peasant_RequestQuit",
-                          TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
-                          TraceLoggingKeyword(TIL_KEYWORD_TRACE));
-    }
-
     void Peasant::AttachContentToWindow(Remoting::AttachRequest request)
     {
         try
diff --git a/src/cascadia/Remoting/Peasant.h b/src/cascadia/Remoting/Peasant.h
index dc860ba907d..1291c8287b9 100644
--- a/src/cascadia/Remoting/Peasant.h
+++ b/src/cascadia/Remoting/Peasant.h
@@ -56,7 +56,6 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
         void RequestRename(const winrt::Microsoft::Terminal::Remoting::RenameRequestArgs& args);
         void RequestShowNotificationIcon();
         void RequestHideNotificationIcon();
-        void RequestQuitAll();
         void Quit();
 
         void AttachContentToWindow(Remoting::AttachRequest request);
@@ -76,7 +75,6 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
 
         til::typed_event<> ShowNotificationIconRequested;
         til::typed_event<> HideNotificationIconRequested;
-        til::typed_event<> QuitAllRequested;
         til::typed_event<> QuitRequested;
 
         til::typed_event<winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::AttachRequest> AttachRequested;
diff --git a/src/cascadia/Remoting/Peasant.idl b/src/cascadia/Remoting/Peasant.idl
index 3463d561f07..3e01e6a6d3f 100644
--- a/src/cascadia/Remoting/Peasant.idl
+++ b/src/cascadia/Remoting/Peasant.idl
@@ -80,7 +80,6 @@ namespace Microsoft.Terminal.Remoting
 
         void RequestShowNotificationIcon();
         void RequestHideNotificationIcon();
-        void RequestQuitAll();
         void Quit();
 
         void AttachContentToWindow(AttachRequest request);
@@ -95,7 +94,6 @@ namespace Microsoft.Terminal.Remoting
 
         event Windows.Foundation.TypedEventHandler<Object, Object> ShowNotificationIconRequested;
         event Windows.Foundation.TypedEventHandler<Object, Object> HideNotificationIconRequested;
-        event Windows.Foundation.TypedEventHandler<Object, Object> QuitAllRequested;
         event Windows.Foundation.TypedEventHandler<Object, Object> QuitRequested;
 
         event Windows.Foundation.TypedEventHandler<Object, AttachRequest> AttachRequested;
diff --git a/src/cascadia/Remoting/WindowManager.cpp b/src/cascadia/Remoting/WindowManager.cpp
index 88c3a0218d1..a07a3960b41 100644
--- a/src/cascadia/Remoting/WindowManager.cpp
+++ b/src/cascadia/Remoting/WindowManager.cpp
@@ -375,6 +375,18 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
         WindowClosed.raise(s, e);
     }
 
+    void WindowManager::QuitAll()
+    {
+        if (_monarch)
+        {
+            try
+            {
+                _monarch.QuitAll();
+            }
+            CATCH_LOG()
+        }
+    }
+
     void WindowManager::SignalClose(const Remoting::Peasant& peasant)
     {
         if (_monarch)
diff --git a/src/cascadia/Remoting/WindowManager.h b/src/cascadia/Remoting/WindowManager.h
index 6dd935e8a65..936e13bc683 100644
--- a/src/cascadia/Remoting/WindowManager.h
+++ b/src/cascadia/Remoting/WindowManager.h
@@ -32,6 +32,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
         Remoting::Peasant CreatePeasant(const Remoting::WindowRequestedArgs& args);
 
         void SignalClose(const Remoting::Peasant& peasant);
+        void QuitAll();
         void SummonWindow(const Remoting::SummonWindowSelectionArgs& args);
         void SummonAllWindows();
         Windows::Foundation::Collections::IVectorView<winrt::Microsoft::Terminal::Remoting::PeasantInfo> GetPeasantInfos();
diff --git a/src/cascadia/Remoting/WindowManager.idl b/src/cascadia/Remoting/WindowManager.idl
index deaf2b7489e..1f1b4c4345d 100644
--- a/src/cascadia/Remoting/WindowManager.idl
+++ b/src/cascadia/Remoting/WindowManager.idl
@@ -12,6 +12,7 @@ namespace Microsoft.Terminal.Remoting
         Peasant CreatePeasant(WindowRequestedArgs args);
 
         void SignalClose(Peasant p);
+        void QuitAll();
 
         void UpdateActiveTabTitle(String title, Peasant p);
 
diff --git a/src/cascadia/UnitTests_Remoting/RemotingTests.cpp b/src/cascadia/UnitTests_Remoting/RemotingTests.cpp
index ed4a1ae94e5..4078363a9da 100644
--- a/src/cascadia/UnitTests_Remoting/RemotingTests.cpp
+++ b/src/cascadia/UnitTests_Remoting/RemotingTests.cpp
@@ -82,7 +82,6 @@ namespace RemotingUnitTests
         void Summon(const Remoting::SummonWindowBehavior& /*args*/) DIE;
         void RequestShowNotificationIcon() DIE;
         void RequestHideNotificationIcon() DIE;
-        void RequestQuitAll() DIE;
         void Quit() DIE;
         void AttachContentToWindow(Remoting::AttachRequest) DIE;
         void SendContent(winrt::Microsoft::Terminal::Remoting::RequestReceiveContentArgs) DIE;
@@ -94,7 +93,6 @@ namespace RemotingUnitTests
         til::typed_event<winrt::Windows::Foundation::IInspectable, Remoting::SummonWindowBehavior> SummonRequested;
         til::typed_event<> ShowNotificationIconRequested;
         til::typed_event<> HideNotificationIconRequested;
-        til::typed_event<> QuitAllRequested;
         til::typed_event<> QuitRequested;
         til::typed_event<winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::AttachRequest> AttachRequested;
         til::typed_event<winrt::Windows::Foundation::IInspectable, winrt::Microsoft::Terminal::Remoting::RequestReceiveContentArgs> SendContentRequested;
@@ -111,6 +109,7 @@ namespace RemotingUnitTests
         void HandleActivatePeasant(Remoting::WindowActivatedArgs /*args*/) DIE;
         void SummonWindow(Remoting::SummonWindowSelectionArgs /*args*/) DIE;
         void SignalClose(uint64_t /*peasantId*/) DIE;
+        void QuitAll() DIE;
 
         void SummonAllWindows() DIE;
         bool DoesQuakeWindowExist() DIE;
diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp
index 6ecd065f460..05f405d98d0 100644
--- a/src/cascadia/WindowsTerminal/AppHost.cpp
+++ b/src/cascadia/WindowsTerminal/AppHost.cpp
@@ -13,6 +13,8 @@
 
 #include <TerminalThemeHelpers.h>
 
+#include <til/latch.h>
+
 using namespace winrt::Windows::UI;
 using namespace winrt::Windows::UI::Composition;
 using namespace winrt::Windows::UI::Xaml;
@@ -348,9 +350,29 @@ void AppHost::Initialize()
     });
 
     _windowCallbacks.AutomaticShutdownRequested = _window->AutomaticShutdownRequested([this]() {
-        // Raised when the OS is beginning an update of the app. We will quit,
-        // to save our state, before the OS manually kills us.
-        _quit();
+        // This is the WM_ENDSESSION handler.
+        // The event is raised when the user is logged out, because the system is rebooting, etc.
+        // Due to the design of WM_ENDSESSION, returning from the message indicates to the OS that it's fine to
+        // terminate us at any time. Luckily Windows has never heavily relied on message passing or asynchronous
+        // eventing in any of its UI frameworks. It also was clearly impossible to use WaitForMultipleObjects with
+        // bWaitAll=TRUE and a timeout to wait for all applications to exit cleanly.
+        // As such we attempt to synchronously shut down the app here. Otherwise, it could just call _quit().
+
+        const auto state = ApplicationState::SharedInstance();
+
+        state.PersistedWindowLayouts(nullptr);
+
+        // A duplicate of AppHost::_QuitRequested().
+        if (_appLogic && _windowLogic && _appLogic.ShouldUsePersistedLayout())
+        {
+            _windowLogic.PersistState();
+        }
+
+        _windowManager.SignalClose(_peasant);
+        _windowManager.QuitAll();
+
+        // Ensure to write the state.json before we get TerminateProcess()d by the OS (Thanks!).
+        state.Flush();
     });
 
     // Load bearing: make sure the PropertyChanged handler is added before we
@@ -440,7 +462,7 @@ winrt::fire_and_forget AppHost::_quit()
     co_await winrt::resume_background();
 
     ApplicationState::SharedInstance().PersistedWindowLayouts(nullptr);
-    peasant.RequestQuitAll();
+    _windowManager.QuitAll();
 }
 
 void AppHost::_revokeWindowCallbacks()
@@ -1157,25 +1179,32 @@ void AppHost::_IsQuakeWindowChanged(const winrt::Windows::Foundation::IInspectab
 }
 
 // Raised from our Peasant. We handle by propagating the call to our terminal window.
-winrt::fire_and_forget AppHost::_QuitRequested(const winrt::Windows::Foundation::IInspectable&,
-                                               const winrt::Windows::Foundation::IInspectable&)
+void AppHost::_QuitRequested(const winrt::Windows::Foundation::IInspectable&, const winrt::Windows::Foundation::IInspectable&)
 {
-    auto weakThis{ weak_from_this() };
-    // Need to be on the main thread to close out all of the tabs.
-    co_await wil::resume_foreground(_windowLogic.GetRoot().Dispatcher());
+    // We process the shutdown synchronously here, because otherwise the
+    // AutomaticShutdownRequested() logic wouldn't run synchronously either.
+    til::latch latch{ 1 };
 
-    const auto strongThis = weakThis.lock();
-    if (!strongThis)
-    {
-        co_return;
-    }
+    _windowLogic.GetRoot().Dispatcher().RunAsync(winrt::Windows::UI::Core::CoreDispatcherPriority::Normal, [&latch, weakThis = weak_from_this()]() {
+        const auto countDownOnExit = wil::scope_exit([&latch] {
+            latch.count_down();
+        });
 
-    if (_appLogic && _windowLogic && _appLogic.ShouldUsePersistedLayout())
-    {
-        _windowLogic.PersistState();
-    }
+        const auto self = weakThis.lock();
+        if (!self)
+        {
+            return;
+        }
+
+        if (self->_appLogic && self->_windowLogic && self->_appLogic.ShouldUsePersistedLayout())
+        {
+            self->_windowLogic.PersistState();
+        }
+
+        PostQuitMessage(0);
+    });
 
-    PostQuitMessage(0);
+    latch.wait();
 }
 
 // Raised from TerminalWindow. We handle by bubbling the request to the window manager.
diff --git a/src/cascadia/WindowsTerminal/AppHost.h b/src/cascadia/WindowsTerminal/AppHost.h
index cc90c21c1c1..1d3331d3bac 100644
--- a/src/cascadia/WindowsTerminal/AppHost.h
+++ b/src/cascadia/WindowsTerminal/AppHost.h
@@ -123,8 +123,8 @@ class AppHost : public std::enable_shared_from_this<AppHost>
     void _SystemMenuChangeRequested(const winrt::Windows::Foundation::IInspectable& sender,
                                     const winrt::TerminalApp::SystemMenuChangeArgs& args);
 
-    winrt::fire_and_forget _QuitRequested(const winrt::Windows::Foundation::IInspectable& sender,
-                                          const winrt::Windows::Foundation::IInspectable& args);
+    void _QuitRequested(const winrt::Windows::Foundation::IInspectable& sender,
+                        const winrt::Windows::Foundation::IInspectable& args);
 
     void _RequestQuitAll(const winrt::Windows::Foundation::IInspectable& sender,
                          const winrt::Windows::Foundation::IInspectable& args);


Test patch: