Skip to content

Fix WindowEmperor exiting too early#15337

Merged
microsoft-github-policy-service[bot] merged 1 commit intomainfrom
dev/lhecker/15306-shutdown-fix
May 12, 2023
Merged

Fix WindowEmperor exiting too early#15337
microsoft-github-policy-service[bot] merged 1 commit intomainfrom
dev/lhecker/15306-shutdown-fix

Conversation

@lhecker
Copy link
Member

@lhecker lhecker commented May 11, 2023

WindowEmperor would exit as soon as the last window would enter
RundownForExit(), which is too early and triggers leak checks.
This commit splits up the shutdown up into deregistering the window from
the list of windows and into actually decrementing the window count.

Closes #15306

Validation Steps Performed

  • D2D leak warnings seem to disappear ✅

@lhecker lhecker added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. labels May 11, 2023
void RundownForExit();

winrt::Microsoft::Terminal::Remoting::Peasant Peasant();
uint64_t PeasantID();
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 made this change because it avoids exposing the innards of WindowThread. It's technically unrelated.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM love it

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

multiple threads are hard man. Thanks for cleaning up my mess.

void RundownForExit();

winrt::Microsoft::Terminal::Remoting::Peasant Peasant();
uint64_t PeasantID();
Copy link
Member

Choose a reason for hiding this comment

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

LGTM love it

@lhecker lhecker added the AutoMerge Marked for automatic merge by the bot when requirements are met label May 12, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot deleted the dev/lhecker/15306-shutdown-fix branch May 12, 2023 20:15
DHowett pushed a commit that referenced this pull request May 12, 2023
`WindowEmperor` would exit as soon as the last window would enter
`RundownForExit()`, which is too early and triggers leak checks.
This commit splits up the shutdown up into deregistering the window from
the list of windows and into actually decrementing the window count.

Closes #15306

## Validation Steps Performed
* D2D leak warnings seem to disappear ✅

(cherry picked from commit 488de2d)
Service-Card-Id: 89180861
Service-Version: 1.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-UserInterface Issues pertaining to the user interface of the Console or Terminal AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

D2D DEBUG ERROR - Memory leaks detected

3 participants