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.

Sign up for GitHub

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

Jump to bottom

Tabs leak GDI handles on tab title resizes #5949

Closed
RealDolos opened this issue May 17, 2020 · 7 comments · Fixed by #6229
Closed

Tabs leak GDI handles on tab title resizes #5949

RealDolos opened this issue May 17, 2020 · 7 comments · Fixed by #6229
Assignees
@miniksa
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Crash Crashes are real bad news.
Milestone
Terminal v1.1

Comments

@RealDolos
Copy link

Environment

Windows build number: 10.0.18363.0
Windows Terminal version (if applicable): 0.11.1333.0

Steps to reproduce

  1. In the preferences, set "tabWidthMode": "titleLength" or any other configuration that allows for variably-sized tab titles (to trigger this bug)
  2. Run the following in a powershell tab (or anything similar in any type of tab) to trigger lots of tab title resizes
for ($i=0;;++$i) { $host.UI.RawUI.WindowTitle = "oops " + @(Get-Random -Minimum 0.0 -Maximum 100000000.3) + @(Get-Random) + @(Get-Random) + " oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooops"; }

Expected behavior

Windows Terminal should keep running, keep updating the tab title, without leaking GDI handles.

Actual behavior

Windows Terminal keeps allocating new GDI handles (as seen in Task Manager/ProcessHacker) and will eventually crash and just vanish.

The leaked GDI handles will be retained even when the corresponding tab is closed

term resource ex

Please note

It is not enough to just update the title when tabs are in equal mode/are not resized.
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels May 17, 2020
@miniksa
Copy link
Member

miniksa commented May 18, 2020

This is leaking before any of the user mode GDI code is being hit. I see a single DC getting left behind every time. But breakpointing on literally all the GDI Create and Delete functions... the handle is leaked before we even hit our user mode code.

@miniksa
Copy link
Member

miniksa commented May 18, 2020

I think this will have to be found with a KD. Also, I think it's not really our fault as ONE handle is leaked when the tab is stood up by MUX. It's just not leaked again otherwise because it doesn't resize.

@WSLUser
Copy link
Contributor

WSLUser commented May 18, 2020

@miniksa I wonder if that leak exists in 2.4 preview of WinUI. Because this sounds alot like it's coming from there.

@DHowett
Copy link
Member

DHowett commented May 18, 2020

Looks like this is a bug in WIL, actually. I failed to update WIL months ago because of static analysis failures, but in so doing I missed microsoft/wil#100.

@DHowett DHowett added Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news. Area-UserInterface Issues pertaining to the user interface of the Console or Terminal labels May 18, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 18, 2020
@DHowett DHowett added Needs-Tag-Fix Doesn't match tag requirements and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 18, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 18, 2020
@DHowett DHowett added this to the Terminal v1.1 milestone May 18, 2020
@miniksa
Copy link
Member

miniksa commented May 18, 2020

Oof good catch, Dustin.

miniksa added a commit that referenced this issue May 27, 2020
@miniksa
Update wil. Fixes #5949
0e043f4
@miniksa miniksa mentioned this issue May 27, 2020
Merged
6 tasks
@ghost ghost added the In-PR This issue has a related PR label May 27, 2020
@ghost ghost closed this as completed in #6229 Jun 1, 2020
@ghost ghost removed the In-PR This issue has a related PR label Jun 1, 2020
ghost pushed a commit that referenced this issue Jun 1, 2020
@miniksa
Update wil. Fixes GDI handle leak (#6229)
48b3262 
## Summary of the Pull Request
When resizing the window title, a GDI object would be leaked. This has to do with our island message handler using `wil` to track these objects and `wil` having a bug.

## References
microsoft/wil#100

## PR Checklist
* [x] Closes #5949 
* [x] I work here.
* [x] Tested manually
* [x] Doc not required.
* [x] Am core contributor.

## Validation Steps Performed
* [x] Added the GDI Objects column to Task Manager, set the Terminal to use the `titleWidth` size tabs, then changed the title a bunch with PowerShell. Confirmed repro before (increasing GDI count). Confirmed it's gone after (no change to object count).
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Jun 1, 2020
@ghost
Copy link

ghost commented Jun 18, 2020

🎉This issue was addressed in #6229, which has now been successfully released as Windows Terminal Preview v1.1.1671.0.:tada:

Handy links:

DHowett pushed a commit that referenced this issue Jun 24, 2020
@miniksa @DHowett
Update wil. Fixes GDI handle leak (#6229)
c6fc68e 
## Summary of the Pull Request
When resizing the window title, a GDI object would be leaked. This has to do with our island message handler using `wil` to track these objects and `wil` having a bug.

## References
microsoft/wil#100

## PR Checklist
* [x] Closes #5949
* [x] I work here.
* [x] Tested manually
* [x] Doc not required.
* [x] Am core contributor.

## Validation Steps Performed
* [x] Added the GDI Objects column to Task Manager, set the Terminal to use the `titleWidth` size tabs, then changed the title a bunch with PowerShell. Confirmed repro before (increasing GDI count). Confirmed it's gone after (no change to object count).

(cherry picked from commit 48b3262)
@ghost
Copy link

ghost commented Jun 30, 2020

🎉This issue was addressed in #6229, which has now been successfully released as Windows Terminal v1.0.1811.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@miniksa miniksa

Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Crash Crashes are real bad news.
Projects
None yet
Milestone
Terminal v1.1
Development

Successfully merging a pull request may close this issue.

Update wil. Fixes GDI handle leak microsoft/terminal
4 participants
@DHowett @RealDolos @miniksa @WSLUser