Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Theoretical fix for some crashes #15457

Merged
merged 3 commits into from
May 26, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/cascadia/WindowsTerminal/IslandWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,16 @@ void IslandWindow::Close()
// </BODGY>
}

// GH#15454: Unset the user data for the window. This will prevent future
// callbacks that come onto our window message loop from being sent to the
// IslandWindow (or other derived class's) implementation.
//
// Specifically, this prevents a pending coroutine from being able to call
// something like ShowWindow, and have that come back on the IslandWindow
// message loop, where it'll end up asking XAML something that XAML is no
// longer able to answer.
SetWindowLongPtr(_window.get(), GWLP_USERDATA, 0);

if (_source)
{
_source.Close();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized... When we do winrt::detach_abi above, it'll null out _source and so we won't call Close() anymore. Is that intentional or should we just remove Close()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, yea - but that's still only for Windows 10, and I'm blowing that bodge away in #15424 anyways

Expand Down