diff --git a/src/cascadia/LocalTests_TerminalApp/pch.h b/src/cascadia/LocalTests_TerminalApp/pch.h index f82561888b1..25df5f47e9e 100644 --- a/src/cascadia/LocalTests_TerminalApp/pch.h +++ b/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 + // Common includes for most tests: #include "../../inc/conattrs.hpp" #include "../../types/inc/utils.hpp" diff --git a/src/cascadia/TerminalApp/ColorHelper.cpp b/src/cascadia/TerminalApp/ColorHelper.cpp index 7ed3f8a260c..c2dd4c2303c 100644 --- a/src/cascadia/TerminalApp/ColorHelper.cpp +++ b/src/cascadia/TerminalApp/ColorHelper.cpp @@ -1,6 +1,4 @@ -#include "pch.h" #include "ColorHelper.h" -#include using namespace winrt::TerminalApp; diff --git a/src/cascadia/TerminalApp/ColorHelper.h b/src/cascadia/TerminalApp/ColorHelper.h index cbe6582c8d1..e4ab3f42b77 100644 --- a/src/cascadia/TerminalApp/ColorHelper.h +++ b/src/cascadia/TerminalApp/ColorHelper.h @@ -1,7 +1,6 @@ #pragma once -#include "pch.h" -#include +#include namespace winrt::TerminalApp { diff --git a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj index 2b933ddfe91..6e1cd5aaaea 100644 --- a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj +++ b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj @@ -237,7 +237,9 @@ - + + NotUsing + Create diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 06121ab945d..08c51cb730c 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/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: diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index fe6ee7d92a3..d3b03f426fd 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -152,7 +152,7 @@ namespace winrt::TerminalApp::implementation void _Setup(); - std::optional _bellIndicatorTimer; + SafeDispatcherTimer _bellIndicatorTimer; void _BellIndicatorTimerTick(const Windows::Foundation::IInspectable& sender, const Windows::Foundation::IInspectable& e); void _MakeTabViewItem() override; diff --git a/src/cascadia/TerminalApp/Toast.h b/src/cascadia/TerminalApp/Toast.h index 40d3df7cf55..63b2b5a0770 100644 --- a/src/cascadia/TerminalApp/Toast.h +++ b/src/cascadia/TerminalApp/Toast.h @@ -34,5 +34,5 @@ class Toast : public std::enable_shared_from_this private: winrt::Microsoft::UI::Xaml::Controls::TeachingTip _tip; - winrt::Windows::UI::Xaml::DispatcherTimer _timer; + SafeDispatcherTimer _timer; }; diff --git a/src/cascadia/TerminalApp/pch.h b/src/cascadia/TerminalApp/pch.h index 01811ad62f6..8d0338108e3 100644 --- a/src/cascadia/TerminalApp/pch.h +++ b/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 + #include #include // must go after the CoreDispatcher type is defined diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 4c59d027a52..2437be05452 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/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); } diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 441ca5f5763..20449f98129 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/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 _autoScrollingPointerPoint; - Windows::UI::Xaml::DispatcherTimer _autoScrollTimer; + SafeDispatcherTimer _autoScrollTimer; std::optional _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 _cursorTimer; - std::optional _blinkTimer; + SafeDispatcherTimer _cursorTimer; + SafeDispatcherTimer _blinkTimer; winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker; bool _showMarksInScrollbar{ false }; diff --git a/src/cascadia/TerminalControl/pch.h b/src/cascadia/TerminalControl/pch.h index 98f905ef088..f7be9ae1eac 100644 --- a/src/cascadia/TerminalControl/pch.h +++ b/src/cascadia/TerminalControl/pch.h @@ -73,7 +73,8 @@ TRACELOGGING_DECLARE_PROVIDER(g_hTerminalControlProvider); #include #include -#include "ThrottledFunc.h" +#include +#include #include #include // must go after the CoreDispatcher type is defined diff --git a/src/cascadia/WinRTUtils/WinRTUtils.vcxproj b/src/cascadia/WinRTUtils/WinRTUtils.vcxproj index 959daaddf8f..13052c590be 100644 --- a/src/cascadia/WinRTUtils/WinRTUtils.vcxproj +++ b/src/cascadia/WinRTUtils/WinRTUtils.vcxproj @@ -19,6 +19,7 @@ + @@ -52,4 +53,4 @@ - + \ No newline at end of file diff --git a/src/cascadia/WinRTUtils/WinRTUtils.vcxproj.filters b/src/cascadia/WinRTUtils/WinRTUtils.vcxproj.filters index 5e47f13f57a..a1d17001d20 100644 --- a/src/cascadia/WinRTUtils/WinRTUtils.vcxproj.filters +++ b/src/cascadia/WinRTUtils/WinRTUtils.vcxproj.filters @@ -2,21 +2,21 @@ + + - + + - - - diff --git a/src/cascadia/WinRTUtils/inc/SafeDispatcherTimer.h b/src/cascadia/WinRTUtils/inc/SafeDispatcherTimer.h new file mode 100644 index 00000000000..1a61bb254b6 --- /dev/null +++ b/src/cascadia/WinRTUtils/inc/SafeDispatcherTimer.h @@ -0,0 +1,92 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +// Par for the course, 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. +struct SafeDispatcherTimer +{ + SafeDispatcherTimer() = default; + SafeDispatcherTimer(SafeDispatcherTimer const&) = delete; + SafeDispatcherTimer& operator=(SafeDispatcherTimer const&) = delete; + SafeDispatcherTimer(SafeDispatcherTimer&&) = delete; + SafeDispatcherTimer& operator=(SafeDispatcherTimer&&) = delete; + + ~SafeDispatcherTimer() + { + Destroy(); + } + + explicit operator bool() const noexcept + { + return _timer != nullptr; + } + + winrt::Windows::Foundation::TimeSpan Interval() + { + return _getTimer().Interval(); + } + + void Interval(winrt::Windows::Foundation::TimeSpan const& value) + { + _getTimer().Interval(value); + } + + bool IsEnabled() + { + return _timer && _timer.IsEnabled(); + } + + void Tick(winrt::Windows::Foundation::EventHandler const& handler) + { + auto& timer = _getTimer(); + if (_token) + { + timer.Tick(_token); + } + _token = timer.Tick(handler); + } + + void Start() + { + _getTimer().Start(); + } + + void Stop() const + { + if (_timer) + { + _timer.Stop(); + } + } + + void Destroy() + { + if (!_timer) + { + return; + } + + _timer.Stop(); + if (_token) + { + _timer.Tick(_token); + } + _timer = nullptr; + _token = {}; + } + +private: + ::winrt::Windows::UI::Xaml::DispatcherTimer& _getTimer() + { + if (!_timer) + { + _timer = ::winrt::Windows::UI::Xaml::DispatcherTimer{}; + } + return _timer; + } + + ::winrt::Windows::UI::Xaml::DispatcherTimer _timer{ nullptr }; + winrt::event_token _token; +}; diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 7cae9959998..3168011a491 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -438,10 +438,7 @@ void AppHost::Close() // After calling _window->Close() we should avoid creating more WinUI related actions. // I suspect WinUI wouldn't like that very much. As such unregister all event handlers first. _revokers = {}; - if (_frameTimer) - { - _frameTimer.Tick(_frameTimerToken); - } + _frameTimer.Destroy(); _showHideWindowThrottler.reset(); _revokeWindowCallbacks(); @@ -1190,12 +1187,8 @@ void AppHost::_startFrameTimer() // _updateFrameColor, which will actually handle setting the colors. If we // already have a timer, just start that one. - if (_frameTimer == nullptr) - { - _frameTimer = winrt::Windows::UI::Xaml::DispatcherTimer(); - _frameTimer.Interval(FrameUpdateInterval); - _frameTimerToken = _frameTimer.Tick({ this, &AppHost::_updateFrameColor }); - } + _frameTimer.Tick({ this, &AppHost::_updateFrameColor }); + _frameTimer.Interval(FrameUpdateInterval); _frameTimer.Start(); } diff --git a/src/cascadia/WindowsTerminal/AppHost.h b/src/cascadia/WindowsTerminal/AppHost.h index fd912aee67d..992a4402874 100644 --- a/src/cascadia/WindowsTerminal/AppHost.h +++ b/src/cascadia/WindowsTerminal/AppHost.h @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +#pragma once + #include "pch.h" #include "NonClientIslandWindow.h" #include "NotificationIcon.h" @@ -56,7 +58,7 @@ class AppHost : public std::enable_shared_from_this std::shared_ptr> _showHideWindowThrottler; std::chrono::time_point _started; - winrt::Windows::UI::Xaml::DispatcherTimer _frameTimer{ nullptr }; + SafeDispatcherTimer _frameTimer; uint32_t _launchShowWindowCommand{ SW_NORMAL }; @@ -165,7 +167,6 @@ class AppHost : public std::enable_shared_from_this void _updateFrameColor(const winrt::Windows::Foundation::IInspectable&, const winrt::Windows::Foundation::IInspectable&); winrt::event_token _GetWindowLayoutRequestedToken; - winrt::event_token _frameTimerToken; // Helper struct. By putting these all into one struct, we can revoke them // all at once, by assigning _revokers to a fresh Revokers instance. That'll diff --git a/src/cascadia/WindowsTerminal/pch.h b/src/cascadia/WindowsTerminal/pch.h index eafe88dfb6c..5011dbeba61 100644 --- a/src/cascadia/WindowsTerminal/pch.h +++ b/src/cascadia/WindowsTerminal/pch.h @@ -48,7 +48,7 @@ Module Name: #include // Needed just for XamlIslands to work at all: -#include +#include #include #include #include @@ -63,7 +63,7 @@ Module Name: #include #include #include -#include +#include #include #include #include @@ -91,5 +91,7 @@ TRACELOGGING_DECLARE_PROVIDER(g_hWindowsTerminalProvider); #include "til.h" #include "til/mutex.h" +#include + #include #include // must go after the CoreDispatcher type is defined