From 23e4d313d5dae913ed16db3cfdb318a05b63ba6a Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 16 Aug 2022 20:30:03 +0200 Subject: [PATCH] Call UpdatePatternLocations from a background thread (#13758) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have a number of theories why #12607 is happening, one of which is that some GPU drivers somehow rely on Win32 messages or similar which we process on the main thread. If we then try to acquire the console lock on the main thread, while the GPU-driver thread itself is holding that lock, we've got ourselves a deadlock. This PR makes this less likely by running the repeat offender `UpdatePatternLocations` on a background thread instead. We have a number of other locations which acquire the console lock on the main thread and a thorough bug fix must be done in a different way. ## Validation Steps Performed * After pasting an URL it gets underlined on hover ✅ --- src/cascadia/TerminalControl/ControlCore.cpp | 13 +++++++------ src/cascadia/TerminalControl/ControlCore.h | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 2ed9f2d7efe..77678029e98 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -195,11 +195,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation } }); - _updatePatternLocations = std::make_shared>( - _dispatcher, + // NOTE: Calling UpdatePatternLocations from a background + // thread is a workaround for us to hit GH#12607 less often. + _updatePatternLocations = std::make_unique>( UpdatePatternLocationsInterval, [weakThis = get_weak()]() { - if (auto core{ weakThis.get() }; !core->_IsClosing()) + if (auto core{ weakThis.get() }) { core->UpdatePatternLocations(); } @@ -509,7 +510,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // itself - it was initiated by the mouse wheel, or the scrollbar. _terminal->UserScrollViewport(viewTop); - _updatePatternLocations->Run(); + (*_updatePatternLocations)(); } void ControlCore::AdjustOpacity(const double adjustment) @@ -1311,7 +1312,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation } // Additionally, start the throttled update of where our links are. - _updatePatternLocations->Run(); + (*_updatePatternLocations)(); } void ControlCore::_terminalCursorPositionChanged() @@ -1705,7 +1706,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation _terminal->Write(hstr); // Start the throttled update of where our hyperlinks are. - _updatePatternLocations->Run(); + (*_updatePatternLocations)(); } catch (...) { diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index c8ba9b88d7d..cc8ab12523e 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -266,7 +266,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation winrt::Windows::System::DispatcherQueue _dispatcher{ nullptr }; std::shared_ptr> _tsfTryRedrawCanvas; - std::shared_ptr> _updatePatternLocations; + std::unique_ptr> _updatePatternLocations; std::shared_ptr> _updateScrollBar; winrt::fire_and_forget _asyncCloseConnection();