From 6bf1507a6c2fa7bf6ff248fdc15d3e7c0a2b6e83 Mon Sep 17 00:00:00 2001
From: Schuyler Rosefield <Rosefield@users.noreply.github.com>
Date: Tue, 19 Oct 2021 20:12:18 -0400
Subject: [PATCH] Try to fix crash on close with saving enabled (#11440)

Don't crash if we try to save the window layout while we are closing, and try to avoid saving at all.

Might impact #11354

## Detailed Description of the Pull Request / Additional comments
- Revoke the event handler/save throttler so we don't even try to get the window layout when we are closing
- Try to check for nullptrs, but then apply `try {} CATCH_LOG()` liberally

## Validation Steps Performed
The happy path of saving normally is still fine, but I haven't been unlucky enough to trigger the crash myself.
---
 src/cascadia/WindowsTerminal/AppHost.cpp | 52 +++++++++++++++++-------
 src/cascadia/WindowsTerminal/AppHost.h   |  1 +
 2 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp
index c5d7418a0..7b6e74ef3 100644
--- a/src/cascadia/WindowsTerminal/AppHost.cpp
+++ b/src/cascadia/WindowsTerminal/AppHost.cpp
@@ -86,7 +86,7 @@ AppHost::AppHost() noexcept :
     _window->SetAlwaysOnTop(_logic.GetInitialAlwaysOnTop());
     _window->MakeWindow();
 
-    _windowManager.GetWindowLayoutRequested([this](auto&&, const winrt::Microsoft::Terminal::Remoting::GetWindowLayoutArgs& args) {
+    _GetWindowLayoutRequestedToken = _windowManager.GetWindowLayoutRequested([this](auto&&, const winrt::Microsoft::Terminal::Remoting::GetWindowLayoutArgs& args) {
         // The peasants are running on separate threads, so they'll need to
         // swap what context they are in to the ui thread to get the actual layout.
         args.WindowLayoutJsonAsync(_GetWindowLayoutAsync());
@@ -401,23 +401,42 @@ void AppHost::LastTabClosed(const winrt::Windows::Foundation::IInspectable& /*se
         _HideNotificationIconRequested();
     }
 
+    // We don't want to try to save layouts if we are about to close
+    _getWindowLayoutThrottler.reset();
+    _windowManager.GetWindowLayoutRequested(_GetWindowLayoutRequestedToken);
+    // Remove ourself from the list of peasants so that we aren't included in
+    // any future requests. This will also mean we block until any existing
+    // event handler finishes.
+    _windowManager.SignalClose();
+
     _window->Close();
 }
 
 LaunchPosition AppHost::_GetWindowLaunchPosition()
 {
-    // Get the position of the current window. This includes the
-    // non-client already.
-    const auto window = _window->GetWindowRect();
-
-    const auto dpi = _window->GetCurrentDpi();
-    const auto nonClientArea = _window->GetNonClientFrame(dpi);
-
-    // The nonClientArea adjustment is negative, so subtract that out.
-    // This way we save the user-visible location of the terminal.
     LaunchPosition pos{};
-    pos.X = window.left - nonClientArea.left;
-    pos.Y = window.top;
+    // If we started saving before closing, but didn't resume the event handler
+    // until after _window might be a nullptr.
+    if (!_window)
+    {
+        return pos;
+    }
+
+    try
+    {
+        // Get the position of the current window. This includes the
+        // non-client already.
+        const auto window = _window->GetWindowRect();
+
+        const auto dpi = _window->GetCurrentDpi();
+        const auto nonClientArea = _window->GetNonClientFrame(dpi);
+
+        // The nonClientArea adjustment is negative, so subtract that out.
+        // This way we save the user-visible location of the terminal.
+        pos.X = window.left - nonClientArea.left;
+        pos.Y = window.top;
+    }
+    CATCH_LOG();
 
     return pos;
 }
@@ -723,10 +742,15 @@ winrt::Windows::Foundation::IAsyncOperation<winrt::hstring> AppHost::_GetWindowL
 {
     winrt::apartment_context peasant_thread;
 
+    winrt::hstring layoutJson = L"";
     // Use the main thread since we are accessing controls.
     co_await winrt::resume_foreground(_logic.GetRoot().Dispatcher());
-    const auto pos = _GetWindowLaunchPosition();
-    const auto layoutJson = _logic.GetWindowLayoutJson(pos);
+    try
+    {
+        const auto pos = _GetWindowLaunchPosition();
+        layoutJson = _logic.GetWindowLayoutJson(pos);
+    }
+    CATCH_LOG()
 
     // go back to give the result to the peasant.
     co_await peasant_thread;
diff --git a/src/cascadia/WindowsTerminal/AppHost.h b/src/cascadia/WindowsTerminal/AppHost.h
index d51ba227a..149c65705 100644
--- a/src/cascadia/WindowsTerminal/AppHost.h
+++ b/src/cascadia/WindowsTerminal/AppHost.h
@@ -114,4 +114,5 @@ private:
     winrt::event_token _NotificationIconPressedToken;
     winrt::event_token _ShowNotificationIconContextMenuToken;
     winrt::event_token _NotificationIconMenuItemSelectedToken;
+    winrt::event_token _GetWindowLayoutRequestedToken;
 };