From a8aa295757110d7dc93f037c65edbce10a7d8286 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 6 Apr 2021 21:07:49 +0200 Subject: [PATCH] Resolve circular reference in ThrottledFunc (#9729) ## Summary of the Pull Request ThrottledFunc previously created a DispatcherTimer whose Tick callback holds a strong reference to the DispatcherTimer itself. This causes a reference cycle, inadvertently leaking timer instances. ## PR Checklist * [x] Closes #7710 * [x] I work here ## Detailed Description of the Pull Request / Additional comments I've initially wanted to remove the `ThrottledFunc<>` optimization, but it turns out that this causes a 3% slowdown. That's definitely not a lot, but enough that we can just keep the optimization for the time being. I've moved the implementation from the .cpp file into the header regardless since the two implementations are extremely similar and it's easier that way to keep them in line. ## Validation Steps Performed I've ensured that the scrollbar still updates its length when I add new lines to a newly created tab. (cherry picked from commit faf372f1654f67c1f2a4094cd1ca8b24739422d3) --- .../TerminalControl/TerminalControl.vcxproj | 1 - .../TerminalControl/ThrottledFunc.cpp | 54 ------------- src/cascadia/TerminalControl/ThrottledFunc.h | 79 +++++++++++++------ 3 files changed, 55 insertions(+), 79 deletions(-) delete mode 100644 src/cascadia/TerminalControl/ThrottledFunc.cpp diff --git a/src/cascadia/TerminalControl/TerminalControl.vcxproj b/src/cascadia/TerminalControl/TerminalControl.vcxproj index 4d07df1f530..bbdb5854463 100644 --- a/src/cascadia/TerminalControl/TerminalControl.vcxproj +++ b/src/cascadia/TerminalControl/TerminalControl.vcxproj @@ -57,7 +57,6 @@ TermControl.xaml - TSFInputControl.xaml diff --git a/src/cascadia/TerminalControl/ThrottledFunc.cpp b/src/cascadia/TerminalControl/ThrottledFunc.cpp deleted file mode 100644 index e72d86297e6..00000000000 --- a/src/cascadia/TerminalControl/ThrottledFunc.cpp +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -#include "pch.h" - -#include "ThrottledFunc.h" - -using namespace winrt::Windows::Foundation; -using namespace winrt::Windows::UI::Core; -using namespace winrt::Windows::UI::Xaml; - -ThrottledFunc<>::ThrottledFunc(ThrottledFunc::Func func, TimeSpan delay, CoreDispatcher dispatcher) : - _func{ func }, - _delay{ delay }, - _dispatcher{ dispatcher }, - _isRunPending{} -{ -} - -// Method Description: -// - Runs the function later, except if `Run` is called again before -// with a new argument, in which case the request will be ignored. -// - For more information, read the class' documentation. -// - This method is always thread-safe. It can be called multiple times on -// different threads. -// Arguments: -// - -// Return Value: -// - -void ThrottledFunc<>::Run() -{ - if (_isRunPending.test_and_set(std::memory_order_acquire)) - { - // already pending - return; - } - - _dispatcher.RunAsync(CoreDispatcherPriority::Low, [weakThis = this->weak_from_this()]() { - if (auto self{ weakThis.lock() }) - { - DispatcherTimer timer; - timer.Interval(self->_delay); - timer.Tick([=](auto&&...) { - if (auto self{ weakThis.lock() }) - { - timer.Stop(); - self->_isRunPending.clear(std::memory_order_release); - self->_func(); - } - }); - timer.Start(); - } - }); -} diff --git a/src/cascadia/TerminalControl/ThrottledFunc.h b/src/cascadia/TerminalControl/ThrottledFunc.h index c3887e76bdf..e31f37a5e62 100644 --- a/src/cascadia/TerminalControl/ThrottledFunc.h +++ b/src/cascadia/TerminalControl/ThrottledFunc.h @@ -56,27 +56,7 @@ class ThrottledFunc : public std::enable_shared_from_this } } - _dispatcher.RunAsync(CoreDispatcherPriority::Low, [weakThis = this->weak_from_this()]() { - if (auto self{ weakThis.lock() }) - { - DispatcherTimer timer; - timer.Interval(self->_delay); - timer.Tick([=](auto&&...) { - if (auto self{ weakThis.lock() }) - { - timer.Stop(); - - std::optional> args; - { - std::lock_guard guard{ self->_lock }; - self->_pendingRunArgs.swap(args); - } - std::apply(self->_func, args.value()); - } - }); - timer.Start(); - } - }); + _Fire(_delay, _dispatcher, this->weak_from_this()); } // Method Description: @@ -110,12 +90,29 @@ class ThrottledFunc : public std::enable_shared_from_this } private: + static winrt::fire_and_forget _Fire(winrt::Windows::Foundation::TimeSpan delay, winrt::Windows::UI::Core::CoreDispatcher dispatcher, std::weak_ptr weakThis) + { + co_await winrt::resume_after(delay); + co_await winrt::resume_foreground(dispatcher); + + if (auto self{ weakThis.lock() }) + { + std::optional> args; + { + std::lock_guard guard{ self->_lock }; + self->_pendingRunArgs.swap(args); + } + + std::apply(self->_func, args.value()); + } + } + Func _func; winrt::Windows::Foundation::TimeSpan _delay; winrt::Windows::UI::Core::CoreDispatcher _dispatcher; - std::optional> _pendingRunArgs; std::mutex _lock; + std::optional> _pendingRunArgs; }; // Class Description: @@ -129,11 +126,45 @@ class ThrottledFunc<> : public std::enable_shared_from_this> public: using Func = std::function; - ThrottledFunc(Func func, winrt::Windows::Foundation::TimeSpan delay, winrt::Windows::UI::Core::CoreDispatcher dispatcher); + ThrottledFunc(Func func, winrt::Windows::Foundation::TimeSpan delay, winrt::Windows::UI::Core::CoreDispatcher dispatcher) : + _func{ func }, + _delay{ delay }, + _dispatcher{ dispatcher } + { + } - void Run(); + // Method Description: + // - Runs the function later, except if `Run` is called again before + // with a new argument, in which case the request will be ignored. + // - For more information, read the class' documentation. + // - This method is always thread-safe. It can be called multiple times on + // different threads. + // Arguments: + // - + // Return Value: + // - + template + void Run(MakeArgs&&... args) + { + if (!_isRunPending.test_and_set(std::memory_order_relaxed)) + { + _Fire(_delay, _dispatcher, this->weak_from_this()); + } + } private: + static winrt::fire_and_forget _Fire(winrt::Windows::Foundation::TimeSpan delay, winrt::Windows::UI::Core::CoreDispatcher dispatcher, std::weak_ptr weakThis) + { + co_await winrt::resume_after(delay); + co_await winrt::resume_foreground(dispatcher); + + if (auto self{ weakThis.lock() }) + { + self->_isRunPending.clear(std::memory_order_relaxed); + self->_func(); + } + } + Func _func; winrt::Windows::Foundation::TimeSpan _delay; winrt::Windows::UI::Core::CoreDispatcher _dispatcher;