Memory leak and other issues when saving settings

[mimvdb]

Windows Terminal version (or Windows build number)

1.8.1521.0 (also observed on source build 2021-07-11)

Other Software

No response

Steps to reproduce

Open the settings dialog and spam click save. No option needs to be changed.

Video: 13 seconds in, the settings could not be reloaded from file (not sure if it is not loading or file corruption).
https://user-images.githubusercontent.com/2277504/125179830-a31f3a00-e1f2-11eb-8b6f-6f6aa07896c5.mp4

Expected Behavior

Memory stays roughly the same, settings file can be loaded after saving, main UI does not extend past its borders.

Actual Behavior

Memory grows and is not reclaimed, settings file can sometimes not be loaded after saving, UI grows past its borders during reloading.

(Second and third only discovered after deciding to record a video for the memory leak)

Not sure if you want separate tracking issues for these, let me know.

[zadjii-msft]

Not sure if you want separate tracking issues for these, let me know.

Typically, yea, we'd prefer separate issues, but since you've already got #10619 open for the overflow bit, we can just leave this open for the memory leak. I've got a strong suspicion that the memory leak and the "failed to load settings" dialog will end up with the same root cause ☺️

[mimvdb]

Unfortunately the question here seems to be what doesn't leak.

• The settings.Copy() doesn't seem to leak 😄
• Determining selectedItemTag

Besides that pretty much any interaction the MainPage::UpdateSettings has with the UI causes the growing memory as seen in the video. Some particulars:

• Any call to _Navigate seems to leak, setting the CacheSize of the contentFrame to 0 doesn't help
• Calls that manipulate the menuItems such as menuItems.ReplaceAll or Clear/Append leak even when the same elements are used
• Calling ReplaceAll twice or Clear/Appending multiple times back to back causes a crash for some reason

I'm stopping my investigation here. Perhaps it is easier to debug with access to the Windows.UI.* source if the terminal team has access?

[mimvdb]

I couldn't resist taking another look. It seems the main (or only) issue is navigating the pages. (The menuItems.ReplaceAll might implicitly navigate, and calling it twice crashing might be an unrelated issue)

Interestingly, some pages leak way more than others. For example, on 1.8.1521.0, the "color schemes" page leaks hardly anything while the "actions" page seems to leak more than a megabyte per click on "discard".

A debug build on main shows "color schemes" also leaking, and makes the "actions" page leak about 5MB per navigation.

I'll file a separate issue for the settings file not loading, as that seems less related now.

[zadjii-msft]

Maybe related:

• Memory leak in NavigationView even though I cleared the child page element while unloading. microsoft-ui-xaml#5978
• [MUXC, WUXC] Memory leak in NavigationView microsoft-ui-xaml#1000 (but that was fixed in 2.2 so I doubt it's that)

I could have sworn I saw another related issue yesterday, but I'm having a hard time finding it now

[zadjii-msft]

Okay, I've got a VS diag session in D:\dev\gh-10609\Report20220316-1134.diagsession. I'm not totally sure how to read this, so bear with me:

Looks like half of the 8MB that got allocated by saving the menu a few times were random ntdll allocations. The other half look like TermControls, so maybe we're accidentally leaking the preview controls somehow. Where are we taking a strong ref when it should be a weak...?

[zadjii-msft]

1005e0d makes this a bit better, but there's still some leakage. Here's the deal though - there's leakage even in the XAML Controls Gallery with the nav view.

[zadjii-msft]

So in this comparison, there was +3MB in DispatchMessageWorker, so I click on that frame, and then below that shows a bunch of RtlpAllocateHeapInternal calls, that all expando to point at Windows.UI.Xaml.dll. So, it's not us?

[zadjii-msft]

"D:\dev\gh-10609\Report20220316-1640 (2).diagsession" has two snapshots, both with +780KB on a single save of the SUI.

[zadjii-msft]

Seems like this is still present on 1.22 canary builds. Even after closing the SUI, the XAML objects remain leaked.

Amazingly enough, I think this might be fixed in WinUI 3. The WinUI 2 gallery repros this pretty readily, just switching pages in the NavView page. But the WinUI 3 gallery didn't seem to.