Skip to content

Commit

Permalink
Call UpdatePatternLocations from a background thread (#13758)
Browse files Browse the repository at this point in the history
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 ✅
  • Loading branch information
lhecker authored Aug 16, 2022
1 parent 96eeac1 commit 23e4d31
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 7 deletions.
13 changes: 7 additions & 6 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
});

_updatePatternLocations = std::make_shared<ThrottledFuncTrailing<>>(
_dispatcher,
// NOTE: Calling UpdatePatternLocations from a background
// thread is a workaround for us to hit GH#12607 less often.
_updatePatternLocations = std::make_unique<til::throttled_func_trailing<>>(
UpdatePatternLocationsInterval,
[weakThis = get_weak()]() {
if (auto core{ weakThis.get() }; !core->_IsClosing())
if (auto core{ weakThis.get() })
{
core->UpdatePatternLocations();
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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 (...)
{
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/ControlCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

winrt::Windows::System::DispatcherQueue _dispatcher{ nullptr };
std::shared_ptr<ThrottledFuncTrailing<>> _tsfTryRedrawCanvas;
std::shared_ptr<ThrottledFuncTrailing<>> _updatePatternLocations;
std::unique_ptr<til::throttled_func_trailing<>> _updatePatternLocations;
std::shared_ptr<ThrottledFuncTrailing<Control::ScrollPositionChangedArgs>> _updateScrollBar;

winrt::fire_and_forget _asyncCloseConnection();
Expand Down

0 comments on commit 23e4d31

Please sign in to comment.