Skip to content

Commit

Permalink
Remove the window thread from the list of threads before nulling the …
Browse files Browse the repository at this point in the history
…AppHost (#15231)

See
#14957 (comment).

I think there's a race here that lets the WindowEmperor muck around with
the window after it's done, but before we remove it from our list of
threads.

This _should_ remove the thread from the list, _then_ null out the
AppHost, then flush the XAML queue, preventing the A/V.

Closes MSFT:43995981
  • Loading branch information
zadjii-msft authored Apr 25, 2023
1 parent 0d1540b commit e491141
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 3 deletions.
12 changes: 11 additions & 1 deletion src/cascadia/WindowsTerminal/WindowEmperor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs&
std::thread t([weakThis, window]() {
try
{
const auto cleanup = wil::scope_exit([&]() {
auto cleanup = wil::scope_exit([&]() {
if (auto self{ weakThis.lock() })
{
self->_windowExitedHandler(window->Peasant().GetID());
Expand All @@ -155,6 +155,16 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs&
}

window->RunMessagePump();

// Manually trigger the cleanup callback. This will ensure that we
// remove the window from our list of windows, before we release the
// AppHost (and subsequently, the host's Logic() member that we use
// elsewhere).
cleanup.reset();

// Now that we no longer care about this thread's window, let it
// release it's app host and flush the rest of the XAML queue.
window->RundownForExit();
}
CATCH_LOG()
});
Expand Down
6 changes: 4 additions & 2 deletions src/cascadia/WindowsTerminal/WindowThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ int WindowThread::RunMessagePump()
// Enter the main window loop.
const auto exitCode = _messagePump();
// Here, the main window loop has exited.
return exitCode;
}

void WindowThread::RundownForExit()
{
_host = nullptr;
// !! LOAD BEARING !!
//
Expand All @@ -54,8 +58,6 @@ int WindowThread::RunMessagePump()
::DispatchMessageW(&msg);
}
}

return exitCode;
}

winrt::TerminalApp::TerminalWindow WindowThread::Logic()
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/WindowsTerminal/WindowThread.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class WindowThread : public std::enable_shared_from_this<WindowThread>
winrt::TerminalApp::TerminalWindow Logic();
void CreateHost();
int RunMessagePump();
void RundownForExit();

winrt::Microsoft::Terminal::Remoting::Peasant Peasant();

Expand Down

0 comments on commit e491141

Please sign in to comment.