Properly manage teardown state in TermControl (#1199)

* Properly manage teardown state in TermControl

This commit introduces a few automatic event revokers and implements
staged Close for TermControl's constituent components.

* Only read the focused control title if there is one

Fixes #1198
Fixes #1188

src/cascadia/TerminalApp/App.cpp

@@ -1036,7 +1036,10 @@ namespace winrt::TerminalApp::implementation
             {
                 try
                 {
-                    return _GetFocusedControl().Title();
+                    if (auto focusedControl{ _GetFocusedControl() })
+                    {
+                        return focusedControl.Title();
+                    }
                 }
                 CATCH_LOG();
             }

src/cascadia/TerminalControl/TermControl.cpp

@@ -80,12 +80,12 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         // Create the SwapChainPanel that will display our content
         Controls::SwapChainPanel swapChainPanel;
 
-        swapChainPanel.SizeChanged({ this, &TermControl::_SwapChainSizeChanged });
+        _sizeChangedRevoker = swapChainPanel.SizeChanged(winrt::auto_revoke, { this, &TermControl::_SwapChainSizeChanged });
-        swapChainPanel.CompositionScaleChanged({ this, &TermControl::_SwapChainScaleChanged });
+        _compositionScaleChangedRevoker = swapChainPanel.CompositionScaleChanged(winrt::auto_revoke, { this, &TermControl::_SwapChainScaleChanged });
 
         // Initialize the terminal only once the swapchainpanel is loaded - that
         //      way, we'll be able to query the real pixel size it got on layout
-        swapChainPanel.Loaded([this] (auto /*s*/, auto /*e*/){
+        _loadedRevoker = swapChainPanel.Loaded(winrt::auto_revoke, [this] (auto /*s*/, auto /*e*/){
             _InitializeTerminal();
         });
 
@@ -355,30 +355,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
 
     TermControl::~TermControl()
     {
-        _closing = true;
+        Close();
-        // Don't let anyone else do something to the buffer.
-        auto lock = _terminal->LockForWriting();
-
-        // Clear out the cursor timer, so it doesn't trigger again on us once we're destructed.
-        if (_cursorTimer)
-        {
-            _cursorTimer.value().Stop();
-            _cursorTimer = std::nullopt;
-        }
-
-        if (_connection)
-        {
-            _connection.Close();
-        }
-
-        if (_renderer)
-        {
-            _renderer->TriggerTeardown();
-        }
-
-        _swapChainPanel = nullptr;
-        _root = nullptr;
-        _connection = nullptr;
     }
 
     UIElement TermControl::GetRoot()
@@ -569,8 +546,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
             _cursorTimer = std::nullopt;
         }
 
-        _controlRoot.GotFocus({ this, &TermControl::_GotFocusHandler });
+        _gotFocusRevoker = _controlRoot.GotFocus(winrt::auto_revoke, { this, &TermControl::_GotFocusHandler });
-        _controlRoot.LostFocus({ this, &TermControl::_LostFocusHandler });
+        _lostFocusRevoker = _controlRoot.LostFocus(winrt::auto_revoke, { this, &TermControl::_LostFocusHandler });
 
         // Focus the control here. If we do it up above (in _Create_), then the
         //      focus won't actually get passed to us. I believe this is because
@@ -1194,9 +1171,39 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
 
     void TermControl::Close()
     {
-        if (!_closing)
+        if (!_closing.exchange(true))
         {
-            this->~TermControl();
+            // Stop accepting new output before we disconnect everything.
+            _connection.TerminalOutput(_connectionOutputEventToken);
+
+            // Clear out the cursor timer, so it doesn't trigger again on us once we're destructed.
+            if (auto localCursorTimer{std::exchange(_cursorTimer, std::nullopt)})
+            {
+                localCursorTimer->Stop();
+                // cursorTimer timer, now stopped, is destroyed.
+            }
+
+            if (auto localConnection{std::exchange(_connection, nullptr)})
+            {
+                localConnection.Close();
+                // connection is destroyed.
+            }
+
+            if (auto localRenderEngine{std::exchange(_renderEngine, nullptr)})
+            {
+                if (auto localRenderer{std::exchange(_renderer, nullptr)})
+                {
+                    localRenderer->TriggerTeardown();
+                    // renderer is destroyed
+                }
+                // renderEngine is destroyed
+            }
+
+            if (auto localTerminal{std::exchange(_terminal, nullptr)})
+            {
+                _initializedTerminal = false;
+                // terminal is destroyed.
+            }
         }
     }
 

src/cascadia/TerminalControl/TermControl.h

@@ -78,7 +78,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
 
         Settings::IControlSettings _settings;
         bool _focused;
-        bool _closing;
+        std::atomic<bool> _closing;
 
         FontInfoDesired _desiredFont;
         FontInfo _actualFont;
@@ -94,6 +94,14 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         //      viewport via touch input.
         std::optional<winrt::Windows::Foundation::Point> _touchAnchor;
 
+        // Event revokers -- we need to deregister ourselves before we die,
+        // lest we get callbacks afterwards.
+        winrt::Windows::UI::Xaml::Controls::Control::SizeChanged_revoker _sizeChangedRevoker;
+        winrt::Windows::UI::Xaml::Controls::SwapChainPanel::CompositionScaleChanged_revoker _compositionScaleChangedRevoker;
+        winrt::Windows::UI::Xaml::Controls::SwapChainPanel::Loaded_revoker _loadedRevoker;
+        winrt::Windows::UI::Xaml::UIElement::LostFocus_revoker _lostFocusRevoker;
+        winrt::Windows::UI::Xaml::UIElement::GotFocus_revoker _gotFocusRevoker;
+
         void _Create();
         void _ApplyUISettings();
         void _InitializeBackgroundBrush();