Skip to content

Commit

Permalink
AGAIN, intentionally leak our App, so that we DON'T crash on exit (#1…
Browse files Browse the repository at this point in the history
…5451)

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**)
  • Loading branch information
zadjii-msft authored May 26, 2023
1 parent 3c3b1aa commit aa8ed8c
Showing 1 changed file with 14 additions and 18 deletions.
32 changes: 14 additions & 18 deletions src/cascadia/WindowsTerminal/WindowEmperor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit aa8ed8c

Please sign in to comment.