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

Severe Memory Leak, Possibly Related to UIA #16209

Closed
Simon818 opened this issue Oct 21, 2023 · 5 comments
Closed

Severe Memory Leak, Possibly Related to UIA #16209

Simon818 opened this issue Oct 21, 2023 · 5 comments
Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing.

Comments

@Simon818
Copy link

Windows Terminal version

1.19.2682.0

Windows build number

10.0.19045.3570

Other Software

NVDA 2023.3

Steps to reproduce

With NVDA running, open several terminal windows and leave them running for a few hours. Occasionally check the usage of the WindowsTerminal.exe process. The terminal windows do not need to be particularly active, though they should have something more than just a command prompt window running on them--perhaps SSH sessions or WSL windows.

Expected Behavior

Windows Terminal should remain within reasonable memory usage.

Actual Behavior

On my system, Windows Terminal's memory usage grows to hundreds of megabytes over time, causing responsiveness issues in Terminal and seemingly affecting the responsiveness of NVDA's UIA access. If I leave a terminal window open overnight, it and all other Terminal windows are nearly unusable the next morning, and the responsiveness of NVDA itself is affected, leaving core OS functionality unusable until I close the process. This leads me to believe the issue is related to UIA.

CC @carlos-zamora @codeofdusk

@Simon818 Simon818 added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 21, 2023
@lhecker
Copy link
Member

lhecker commented Oct 21, 2023

Possible cause: This doesn't release any memory
https://github.com/microsoft/terminal/blob/main/src/renderer/uia/UiaRenderer.cpp#L234

If it's due to that, we could limit to either 4KiB or 2x the previous write size, whatever is larger (as an example):

if (_newOutput.capacity() >= std::max<size_t>(4096, _queuedOutput.size() * 2))
{
    _newOutput = std::wstring{};
    _newOutput.reserve(2048);
}

@lhecker lhecker added Area-Accessibility Issues related to accessibility Product-Terminal The new Windows Terminal. Priority-2 A description (P2) labels Oct 24, 2023
@carlos-zamora
Copy link
Member

Thanks @Simon818. Could you capture a memory dump for us? We have a guide for that right here: https://github.com/microsoft/terminal/wiki/Troubleshooting-Tips#capturing-and-sending-dumps

@carlos-zamora carlos-zamora added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 24, 2023
@carlos-zamora carlos-zamora added this to the Terminal v1.20 milestone Oct 24, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Oct 28, 2023
@zadjii-msft
Copy link
Member

I'm gonna give @Simon818 another week to try and get back to us here. This sounds really painful, but we need to get a dump to see where these allocations are coming from.

I suppose @carlos-zamora you might also be able to use App Verifier pointed at the Terminal with NVDA turned on. That might catch some of the more obvious mistakes 🤷

@microsoft-github-policy-service microsoft-github-policy-service bot removed the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Oct 30, 2023
@zadjii-msft
Copy link
Member

I'm gonna close this in favor of #16217, since that thread has more of an investigation in it. Thanks!

/dup #16217

Copy link
Contributor

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!

@microsoft-github-policy-service microsoft-github-policy-service bot removed this from the Terminal v1.20 milestone Nov 1, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Accessibility Issues related to accessibility Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing.
Projects
None yet
Development

No branches or pull requests

4 participants