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

Conversation

zadjii-msft
Copy link
Member

RE:

I think this should fix all of those, but I want to ship and verify live, since I can't repro this locally.

// message loop, where it'll end up asking XAML something that XAML is no
// longer able to answer.
SetWindowLongPtr(_window.get(), GWLP_USERDATA, (LONG_PTR)nullptr);

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

@zadjii-msft zadjii-msft requested a review from DHowett May 26, 2023 18:42
@DHowett DHowett merged commit 8611d90 into main May 26, 2023
@DHowett DHowett deleted the dev/migrie/b/15454-like-whats-that-gonna-do branch May 26, 2023 19:31
DHowett pushed a commit that referenced this pull request May 26, 2023
RE:
* #15454
* MSFT:44725712 "WindowsTerminal.exe!NonClientIslandWindow::OnSize"
* MSFT:44754014 "NonClientIslandWindow::GetTotalNonClientExclusiveSize"

I think this should fix all of those, but I want to ship and verify
live, since I can't repro this locally.

(cherry picked from commit 8611d90)
Service-Card-Id: 89337669
Service-Version: 1.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants