From 038ad3b50980e065c74565c0d62426ee553c5e69 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 18 Aug 2022 18:48:37 -0500 Subject: [PATCH] Fix a crash in `_WritePseudoWindowCallback` (#13777) Fixes MSFT:40853556 There's a small race here. The renderer thread in ConPTY might notice the terminal is gone, call CloseOutput, and release the vt renderer, and then the window proc fires and decides to minimize/restore the window, triggering an A/V. I'm 100% confident that this has NEVER happened to a real user. But the test labs hit it so much that it makes up ~26% of our crashes. I haven't tested this cause again, _it doesn't hit in the wild_ (cherry picked from commit f58240c9c03a889e9cab9472092da7024b833419) Service-Card-Id: 85103518 Service-Version: 1.15 --- src/host/VtIo.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp index b8d817d34f..71137cefee 100644 --- a/src/host/VtIo.cpp +++ b/src/host/VtIo.cpp @@ -258,8 +258,20 @@ bool VtIo::IsUsingVt() const g.getConsoleInformation().GetActiveOutputBuffer().SetTerminalConnection(_pVtRenderEngine.get()); g.getConsoleInformation().GetActiveInputBuffer()->SetTerminalConnection(_pVtRenderEngine.get()); ServiceLocator::SetPseudoWindowCallback([&](bool showOrHide) -> void { - // Set the remote window visibility to the request - LOG_IF_FAILED(_pVtRenderEngine->SetWindowVisibility(showOrHide)); + // MSFT:40853556 Grab the shutdown lock here, so that another + // thread can't trigger a CloseOutput and release the + // _pVtRenderEngine out from underneath us. + // + // I'm doing this instead of just + // SetPseudoWindowCallback(nullptr), so that we don't just move + // the A/V race to in between checking + // _pseudoWindowMessageCallback and actually calling it. + std::lock_guard lk(_shutdownLock); + if (_pVtRenderEngine) + { + // Set the remote window visibility to the request + LOG_IF_FAILED(_pVtRenderEngine->SetWindowVisibility(showOrHide)); + } }); } CATCH_RETURN();