From d1ac220850d8f467f215958057b45c7762046954 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Mon, 1 Feb 2021 00:26:46 +0000 Subject: [PATCH 1/2] Lock terminal while closing to prevent a crash. --- src/cascadia/TerminalControl/TermControl.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 0a6b453e8ee..cfd159d519d 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -2598,6 +2598,16 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // closed. We can just do it whenever. _AsyncCloseConnection(); + { + // We lock the terminal here to make sure it isn't still being + // used in the connection thread before we destroy the renderer. + // However, we must unlock it again prior to triggering the + // teardown, to avoid the render thread being deadlocked. The + // renderer may be waiting to acquire the terminal lock, while + // we're waiting for the renderer to finish. + auto lock = _terminal->LockForWriting(); + } + if (auto localRenderEngine{ std::exchange(_renderEngine, nullptr) }) { if (auto localRenderer{ std::exchange(_renderer, nullptr) }) From ad50651346f2fbf3d6c8544de410d5d024d1ffe5 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Mon, 1 Feb 2021 19:37:52 +0000 Subject: [PATCH 2/2] Add issue number to the comment. Co-authored-by: Mike Griese --- src/cascadia/TerminalControl/TermControl.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index cfd159d519d..718f0ee07d4 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -2599,6 +2599,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation _AsyncCloseConnection(); { + // GH#8734: // We lock the terminal here to make sure it isn't still being // used in the connection thread before we destroy the renderer. // However, we must unlock it again prior to triggering the