Avoid timer ticks on frozen windows (#16587)

At the time of writing, closing the last tab of a window inexplicably
doesn't lead to the destruction of the remaining TermControl instance.
On top of that, on Win10 we don't destroy window threads due to bugs in
DesktopWindowXamlSource. In other words, we leak TermControl instances.

Additionally, the XAML timer class is "self-referential".
Releasing all references to an instance will not stop the timer.
Only calling Stop() explicitly will achieve that.

The result is that the message loop of a frozen window thread has so
far received 1-2 messages per second due to the blink timer not being
stopped. This may have filled the message queue and lead to bugs as
described in #16332 where keyboard input stopped working.

src/cascadia/LocalTests_TerminalApp/pch.h

@@ -68,6 +68,8 @@ Author(s):
 // Manually include til after we include Windows.Foundation to give it winrt superpowers
 #include "til.h"
 
+#include <SafeDispatcherTimer.h>
+
 // Common includes for most tests:
 #include "../../inc/conattrs.hpp"
 #include "../../types/inc/utils.hpp"

src/cascadia/TerminalApp/ColorHelper.cpp

@@ -1,6 +1,4 @@
-#include "pch.h"
 #include "ColorHelper.h"
-#include <limits>
 
 using namespace winrt::TerminalApp;
 

src/cascadia/TerminalApp/ColorHelper.h

@@ -1,7 +1,6 @@
 #pragma once
-#include "pch.h"
 
-#include <winrt/windows.ui.core.h>
+#include <winrt/Windows.UI.h>
 
 namespace winrt::TerminalApp
 {

src/cascadia/TerminalApp/TerminalAppLib.vcxproj

@@ -237,7 +237,9 @@
     </ClCompile>
     <ClCompile Include="Pane.cpp" />
     <ClCompile Include="Pane.LayoutSizeNode.cpp" />
-    <ClCompile Include="ColorHelper.cpp" />
+    <ClCompile Include="ColorHelper.cpp">
+      <PrecompiledHeader>NotUsing</PrecompiledHeader>
+    </ClCompile>
     <ClCompile Include="DebugTapConnection.cpp" />
     <ClCompile Include="pch.cpp">
       <PrecompiledHeader>Create</PrecompiledHeader>

src/cascadia/TerminalApp/TerminalTab.cpp

@@ -121,12 +121,7 @@ namespace winrt::TerminalApp::implementation
     void TerminalTab::_BellIndicatorTimerTick(const Windows::Foundation::IInspectable& /*sender*/, const Windows::Foundation::IInspectable& /*e*/)
     {
         ShowBellIndicator(false);
-        // Just do a sanity check that the timer still exists before we stop it
-        if (_bellIndicatorTimer.has_value())
-        {
-            _bellIndicatorTimer->Stop();
-            _bellIndicatorTimer = std::nullopt;
-        }
+        _bellIndicatorTimer.Stop();
     }
 
     // Method Description:
@@ -356,14 +351,13 @@ namespace winrt::TerminalApp::implementation
     {
         ASSERT_UI_THREAD();
 
-        if (!_bellIndicatorTimer.has_value())
+        if (!_bellIndicatorTimer)
         {
-            DispatcherTimer bellIndicatorTimer;
-            bellIndicatorTimer.Interval(std::chrono::milliseconds(2000));
-            bellIndicatorTimer.Tick({ get_weak(), &TerminalTab::_BellIndicatorTimerTick });
-            bellIndicatorTimer.Start();
-            _bellIndicatorTimer.emplace(std::move(bellIndicatorTimer));
+            _bellIndicatorTimer.Interval(std::chrono::milliseconds(2000));
+            _bellIndicatorTimer.Tick({ get_weak(), &TerminalTab::_BellIndicatorTimerTick });
         }
+
+        _bellIndicatorTimer.Start();
     }
 
     // Method Description:

src/cascadia/TerminalApp/TerminalTab.h

@@ -152,7 +152,7 @@ namespace winrt::TerminalApp::implementation
 
         void _Setup();
 
-        std::optional<Windows::UI::Xaml::DispatcherTimer> _bellIndicatorTimer;
+        SafeDispatcherTimer _bellIndicatorTimer;
         void _BellIndicatorTimerTick(const Windows::Foundation::IInspectable& sender, const Windows::Foundation::IInspectable& e);
 
         void _MakeTabViewItem() override;

src/cascadia/TerminalApp/Toast.h

@@ -34,5 +34,5 @@ class Toast : public std::enable_shared_from_this<Toast>
 
 private:
     winrt::Microsoft::UI::Xaml::Controls::TeachingTip _tip;
-    winrt::Windows::UI::Xaml::DispatcherTimer _timer;
+    SafeDispatcherTimer _timer;
 };

src/cascadia/TerminalApp/pch.h

@@ -83,6 +83,8 @@ TRACELOGGING_DECLARE_PROVIDER(g_hTerminalAppProvider);
 // Manually include til after we include Windows.Foundation to give it winrt superpowers
 #include "til.h"
 
+#include <SafeDispatcherTimer.h>
+
 #include <cppwinrt_utils.h>
 #include <wil/cppwinrt_helpers.h> // must go after the CoreDispatcher type is defined
 

src/cascadia/TerminalControl/TermControl.cpp

@@ -57,10 +57,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _isInternalScrollBarUpdate{ false },
         _autoScrollVelocity{ 0 },
         _autoScrollingPointerPoint{ std::nullopt },
-        _autoScrollTimer{},
         _lastAutoScrollUpdateTime{ std::nullopt },
-        _cursorTimer{},
-        _blinkTimer{},
         _searchBox{ nullptr }
     {
         InitializeComponent();
@@ -1087,10 +1084,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         if (blinkTime != INFINITE)
         {
             // Create a timer
-            DispatcherTimer cursorTimer;
-            cursorTimer.Interval(std::chrono::milliseconds(blinkTime));
-            cursorTimer.Tick({ get_weak(), &TermControl::_CursorTimerTick });
-            _cursorTimer.emplace(std::move(cursorTimer));
+            _cursorTimer.Interval(std::chrono::milliseconds(blinkTime));
+            _cursorTimer.Tick({ get_weak(), &TermControl::_CursorTimerTick });
             // As of GH#6586, don't start the cursor timer immediately, and
             // don't show the cursor initially. We'll show the cursor and start
             // the timer when the control is first focused.
@@ -1105,13 +1100,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             _core.CursorOn(_focused || _displayCursorWhileBlurred());
             if (_displayCursorWhileBlurred())
             {
-                _cursorTimer->Start();
+                _cursorTimer.Start();
             }
         }
         else
         {
-            // The user has disabled cursor blinking
-            _cursorTimer = std::nullopt;
+            _cursorTimer.Destroy();
         }
 
         // Set up blinking attributes
@@ -1120,16 +1114,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         if (animationsEnabled && blinkTime != INFINITE)
         {
             // Create a timer
-            DispatcherTimer blinkTimer;
-            blinkTimer.Interval(std::chrono::milliseconds(blinkTime));
-            blinkTimer.Tick({ get_weak(), &TermControl::_BlinkTimerTick });
-            blinkTimer.Start();
-            _blinkTimer.emplace(std::move(blinkTimer));
+            _blinkTimer.Interval(std::chrono::milliseconds(blinkTime));
+            _blinkTimer.Tick({ get_weak(), &TermControl::_BlinkTimerTick });
+            _blinkTimer.Start();
         }
         else
         {
             // The user has disabled blinking
-            _blinkTimer = std::nullopt;
+            _blinkTimer.Destroy();
         }
 
         // Now that the renderer is set up, update the appearance for initialization
@@ -1498,7 +1490,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             // Manually show the cursor when a key is pressed. Restarting
             // the timer prevents flickering.
             _core.CursorOn(_core.SelectionMode() != SelectionInteractionMode::Mark);
-            _cursorTimer->Start();
+            _cursorTimer.Start();
         }
 
         return handled;
@@ -1973,12 +1965,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         {
             // When the terminal focuses, show the cursor immediately
             _core.CursorOn(_core.SelectionMode() != SelectionInteractionMode::Mark);
-            _cursorTimer->Start();
+            _cursorTimer.Start();
         }
 
         if (_blinkTimer)
         {
-            _blinkTimer->Start();
+            _blinkTimer.Start();
         }
 
         // Only update the appearance here if an unfocused config exists - if an
@@ -2021,13 +2013,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         if (_cursorTimer && !_displayCursorWhileBlurred())
         {
-            _cursorTimer->Stop();
+            _cursorTimer.Stop();
             _core.CursorOn(false);
         }
 
         if (_blinkTimer)
         {
-            _blinkTimer->Stop();
+            _blinkTimer.Stop();
         }
 
         // Check if there is an unfocused config we should set the appearance to
@@ -2278,7 +2270,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
             // Disconnect the TSF input control so it doesn't receive EditContext events.
             TSFInputControl().Close();
+
+            // At the time of writing, closing the last tab of a window inexplicably
+            // does not lead to the destruction of the remaining TermControl instance(s).
+            // On Win10 we don't destroy window threads due to bugs in DesktopWindowXamlSource.
+            // In turn, we leak TermControl instances. This results in constant HWND messages
+            // while the thread is supposed to be idle. Stop these timers avoids this.
             _autoScrollTimer.Stop();
+            _bellLightTimer.Stop();
+            _cursorTimer.Stop();
+            _blinkTimer.Stop();
 
             if (!_detached)
             {
@@ -3129,20 +3130,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             _bellDarkAnimation.Duration(winrt::Windows::Foundation::TimeSpan(std::chrono::milliseconds(TerminalWarningBellInterval)));
         }
 
-        // Similar to the animation, only initialize the timer here
-        if (!_bellLightTimer)
-        {
-            _bellLightTimer = {};
-            _bellLightTimer.Interval(std::chrono::milliseconds(TerminalWarningBellInterval));
-            _bellLightTimer.Tick({ get_weak(), &TermControl::_BellLightOff });
-        }
-
         Windows::Foundation::Numerics::float2 zeroSize{ 0, 0 };
         // If the grid has 0 size or if the bell timer is
         // already active, do nothing
         if (RootGrid().ActualSize() != zeroSize && !_bellLightTimer.IsEnabled())
         {
-            // Start the timer, when the timer ticks we switch off the light
+            _bellLightTimer.Interval(std::chrono::milliseconds(TerminalWarningBellInterval));
+            _bellLightTimer.Tick({ get_weak(), &TermControl::_BellLightOff });
             _bellLightTimer.Start();
 
             // Switch on the light and animate the intensity to fade out
@@ -3162,15 +3156,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     void TermControl::_BellLightOff(const Windows::Foundation::IInspectable& /* sender */,
                                     const Windows::Foundation::IInspectable& /* e */)
     {
-        if (_bellLightTimer)
-        {
-            // Stop the timer and switch off the light
-            _bellLightTimer.Stop();
+        // Stop the timer and switch off the light
+        _bellLightTimer.Stop();
 
-            if (!_IsClosing())
-            {
-                VisualBellLight::SetIsTarget(RootGrid(), false);
-            }
+        if (!_IsClosing())
+        {
+            VisualBellLight::SetIsTarget(RootGrid(), false);
         }
     }
 
@@ -3729,9 +3720,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         {
             // If we should be ALWAYS displaying the cursor, turn it on and start blinking.
             _core.CursorOn(true);
-            if (_cursorTimer.has_value())
+            if (_cursorTimer)
             {
-                _cursorTimer->Start();
+                _cursorTimer.Start();
             }
         }
         else
@@ -3740,9 +3731,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             // blinking. (if we're focused, then we're already doing the right
             // thing)
             const auto focused = FocusState() != FocusState::Unfocused;
-            if (!focused && _cursorTimer.has_value())
+            if (!focused && _cursorTimer)
             {
-                _cursorTimer->Stop();
+                _cursorTimer.Stop();
             }
             _core.CursorOn(focused);
         }

src/cascadia/TerminalControl/TermControl.h

@@ -236,16 +236,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         // viewport. View is then scrolled to 'follow' the cursor.
         double _autoScrollVelocity;
         std::optional<Windows::UI::Input::PointerPoint> _autoScrollingPointerPoint;
-        Windows::UI::Xaml::DispatcherTimer _autoScrollTimer;
+        SafeDispatcherTimer _autoScrollTimer;
         std::optional<std::chrono::high_resolution_clock::time_point> _lastAutoScrollUpdateTime;
         bool _pointerPressedInBounds{ false };
 
         winrt::Windows::UI::Composition::ScalarKeyFrameAnimation _bellLightAnimation{ nullptr };
         winrt::Windows::UI::Composition::ScalarKeyFrameAnimation _bellDarkAnimation{ nullptr };
-        Windows::UI::Xaml::DispatcherTimer _bellLightTimer{ nullptr };
+        SafeDispatcherTimer _bellLightTimer;
 
-        std::optional<Windows::UI::Xaml::DispatcherTimer> _cursorTimer;
-        std::optional<Windows::UI::Xaml::DispatcherTimer> _blinkTimer;
+        SafeDispatcherTimer _cursorTimer;
+        SafeDispatcherTimer _blinkTimer;
 
         winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker;
         bool _showMarksInScrollbar{ false };

src/cascadia/TerminalControl/pch.h

@@ -73,7 +73,8 @@ TRACELOGGING_DECLARE_PROVIDER(g_hTerminalControlProvider);
 #include <til/mutex.h>
 #include <til/winrt.h>
 
-#include "ThrottledFunc.h"
+#include <SafeDispatcherTimer.h>
+#include <ThrottledFunc.h>
 
 #include <cppwinrt_utils.h>
 #include <wil/cppwinrt_helpers.h> // must go after the CoreDispatcher type is defined

src/cascadia/WinRTUtils/WinRTUtils.vcxproj

@@ -19,6 +19,7 @@
   <Import Project="$(OpenConsoleDir)src\cppwinrt.build.pre.props" />
   <!-- ========================= Headers ======================== -->
   <ItemGroup>
+    <ClInclude Include="inc\SafeDispatcherTimer.h" />
     <ClInclude Include="pch.h" />
     <ClInclude Include="inc\ScopedResourceLoader.h" />
     <ClInclude Include="inc\LibraryResources.h" />
@@ -52,4 +53,4 @@
   <!-- ========================= Globals ======================== -->
   <Import Project="$(OpenConsoleDir)src\cppwinrt.build.post.props" />
   <Import Project="$(OpenConsoleDir)src\common.nugetversions.targets" />
-</Project>
+</Project>

src/cascadia/WinRTUtils/WinRTUtils.vcxproj.filters

@@ -2,21 +2,21 @@
 <Project ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
   <ItemGroup>
     <Natvis Include="$(SolutionDir)tools\ConsoleTypes.natvis" />
+    <Natvis Include="$(MSBuildThisFileDirectory)..\..\natvis\wil.natvis" />
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="pch.cpp" />
     <ClCompile Include="LibraryResources.cpp" />
     <ClCompile Include="ScopedResourceLoader.cpp" />
+    <ClCompile Include="Utils.cpp" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="pch.h" />
-    <ClInclude Include="ScopedResourceLoader.h" />
     <ClInclude Include="inc\LibraryResources.h" />
+    <ClInclude Include="inc\SafeDispatcherTimer.h" />
+    <ClInclude Include="inc\ScopedResourceLoader.h" />
     <ClInclude Include="inc\ThrottledFunc.h" />
     <ClInclude Include="inc\Utils.h" />
     <ClInclude Include="inc\WtExeUtils.h" />
   </ItemGroup>
-  <ItemGroup>
-    <None Include="packages.config" />
-  </ItemGroup>
 </Project>