From aa8ed8c2d47aa51a0cfb88aa6b2f4e9a34111cb5 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 26 May 2023 13:09:00 -0500 Subject: [PATCH] AGAIN, intentionally leak our App, so that we DON'T crash on exit (#15451) This is a resurrection of #5629. As it so happens, this crash-on-exit was _not_ specific to my laptop. It's a bug in the XAML platform somewhere, only on Windows 10. In #14843, we moved this leak into `becomeMonarch`. Turns out, we don't just need this leak for the monarch process, but for all of them. It's not a real "leak", because ultimately, our `App` lives for the entire lifetime of our process, and then gets cleaned up when we do. But `dtor`ing the `App` - that's apparently a no-no. Was originally in #15424, but I'm pulling it out for a super-hotfix release. Closes #15410 MSFT:35761869 looks like it was closed as no repro many moons ago. This should close out our hits there (firmly **40% of the crashes we've gotten on 1.18**) --- .../WindowsTerminal/WindowEmperor.cpp | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 77701a354ce..54d5e2d4f4a 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -37,6 +37,20 @@ WindowEmperor::WindowEmperor() noexcept : }); _dispatcher = winrt::Windows::System::DispatcherQueue::GetForCurrentThread(); + + // BODGY + // + // There's a mysterious crash in XAML on Windows 10 if you just let the App + // get dtor'd. By all accounts, it doesn't make sense. To mitigate this, we + // need to intentionally leak a reference to our App. Crazily, if you just + // let the app get cleaned up with the rest of the process when the process + // exits, then it doesn't crash. But if you let it get explicitly dtor'd, it + // absolutely will crash on exit. + // + // GH#15410 has more details. + + auto a{ _app }; + ::winrt::detach_abi(a); } WindowEmperor::~WindowEmperor() @@ -296,24 +310,6 @@ void WindowEmperor::_becomeMonarch() // We want at least some delay to prevent the first save from overwriting _getWindowLayoutThrottler.emplace(std::move(std::chrono::seconds(10)), std::move([this]() { _saveWindowLayoutsRepeat(); })); _getWindowLayoutThrottler.value()(); - - // BODGY - // - // We've got a weird crash that happens terribly inconsistently, but pretty - // readily on migrie's laptop, only in Debug mode. Apparently, there's some - // weird ref-counting magic that goes on during teardown, and our - // Application doesn't get closed quite right, which can cause us to crash - // into the debugger. This of course, only happens on exit, and happens - // somewhere in the XamlHost.dll code. - // - // Crazily, if we _manually leak the Application_ here, then the crash - // doesn't happen. This doesn't matter, because we really want the - // Application to live for _the entire lifetime of the process_, so the only - // time when this object would actually need to get cleaned up is _during - // exit_. So we can safely leak this Application object, and have it just - // get cleaned up normally when our process exits. - auto a{ _app }; - ::winrt::detach_abi(a); } // sender and args are always nullptr