Skip to content

Commit

Permalink
Theoretical fix for some crashes (#16047)
Browse files Browse the repository at this point in the history
Found this while looking through dumps for failure
`f544cf8e-1879-c59b-3f0b-1a364b92b974`. That's MSFT:45210947. (1% of our
1.19 crashes)

From the dump I looked at,

Looks like,

* we're on Windows 10
* We're refrigerating a window
* We are pumping the remaining XAML messages as we refrigerate
(`_pumpRemainingXamlMessages`)
* In there, we're finally getting the
`TerminalPage::_CompleteInitialization`
* that calls up to the `_root->Initialized` lambda set up in
`TerminalWindow::Initialize`
* There it tries to get the launch mode from the settings, and explodes.
Presumably _settings is null, but can't see in this dump.

so the window is closing before it's initialized.

When we `_warmWindow = std::move(_host->Refrigerate())`, we call
`AppHost::Refrigerate`, which will null out the TerminalWindow. So when
we're getting to `TerminalWindow::Initialize`, we're calling that on a
nullptr. That's the trick.

We need to revoke the internal Initialized callback. Which makes sense.
It's a lambda that binds _this_ 🤦

Service-Card-Id: 90878965
Service-Version: 1.18

---

After more looking, it really doesn't _seem_ like the stacks that are
tracked in `f544cf8e-1879-c59b-3f0b-1a364b92b974` look like the same
stack that I was debugging, but this _is_ a realy issue regardless.

(cherry picked from commit 7073ec0)
  • Loading branch information
zadjii-msft authored and DHowett committed Nov 7, 2023
1 parent 32b6220 commit 9d521d7
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 26 deletions.
55 changes: 29 additions & 26 deletions src/cascadia/TerminalApp/TerminalWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,32 +216,7 @@ namespace winrt::TerminalApp::implementation

_root->SetSettings(_settings, false); // We're on our UI thread right now, so this is safe
_root->Loaded({ get_weak(), &TerminalWindow::_OnLoaded });

_root->Initialized([this](auto&&, auto&&) {
// GH#288 - When we finish initialization, if the user wanted us
// launched _fullscreen_, toggle fullscreen mode. This will make sure
// that the window size is _first_ set up as something sensible, so
// leaving fullscreen returns to a reasonable size.
const auto launchMode = this->GetLaunchMode();
if (_WindowProperties->IsQuakeWindow() || WI_IsFlagSet(launchMode, LaunchMode::FocusMode))
{
_root->SetFocusMode(true);
}

// The IslandWindow handles (creating) the maximized state
// we just want to record it here on the page as well.
if (WI_IsFlagSet(launchMode, LaunchMode::MaximizedMode))
{
_root->Maximized(true);
}

if (WI_IsFlagSet(launchMode, LaunchMode::FullscreenMode) && !_WindowProperties->IsQuakeWindow())
{
_root->SetFullscreen(true);
}

AppLogic::Current()->NotifyRootInitialized();
});
_root->Initialized({ get_weak(), &TerminalWindow::_pageInitialized });
_root->Create();

AppLogic::Current()->SettingsChanged({ get_weak(), &TerminalWindow::UpdateSettingsHandler });
Expand All @@ -260,6 +235,34 @@ namespace winrt::TerminalApp::implementation
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
}

void TerminalWindow::_pageInitialized(const IInspectable&, const IInspectable&)
{
// GH#288 - When we finish initialization, if the user wanted us
// launched _fullscreen_, toggle fullscreen mode. This will make sure
// that the window size is _first_ set up as something sensible, so
// leaving fullscreen returns to a reasonable size.
const auto launchMode = this->GetLaunchMode();
if (_WindowProperties->IsQuakeWindow() || WI_IsFlagSet(launchMode, LaunchMode::FocusMode))
{
_root->SetFocusMode(true);
}

// The IslandWindow handles (creating) the maximized state
// we just want to record it here on the page as well.
if (WI_IsFlagSet(launchMode, LaunchMode::MaximizedMode))
{
_root->Maximized(true);
}

if (WI_IsFlagSet(launchMode, LaunchMode::FullscreenMode) && !_WindowProperties->IsQuakeWindow())
{
_root->SetFullscreen(true);
}

AppLogic::Current()->NotifyRootInitialized();
}

void TerminalWindow::Quit()
{
if (_root)
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TerminalWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ namespace winrt::TerminalApp::implementation

void _RefreshThemeRoutine();
void _OnLoaded(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs);
void _pageInitialized(const IInspectable& sender, const IInspectable& eventArgs);
void _OpenSettingsUI();

winrt::Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::ActionAndArgs> _contentStringToActions(const winrt::hstring& content,
Expand Down

0 comments on commit 9d521d7

Please sign in to comment.