microsoft / microsoft-ui-xaml

WinUI: a modern UI framework with a rich set of controls and styles to build dynamic and high-performing Windows applications.
MIT License
6.38k stars 683 forks source link

Window objects do not deregister event handlers added via XAML or implicitly by using x:Bind, leading to memory leaks #9960

Open lhak opened 2 months ago

lhak commented 2 months ago

Describe the bug

As investigated in https://github.com/microsoft/microsoft-ui-xaml/issues/7282, window objects can easily leak in at least two scenarios:

Event handlers in the XAML file of the window

Adding event handlers to the XAML window from XAML prevents deallocation to the window. Registering and deregistering them in code works fine.

Using x:Bind in the XAML file of the window

Adding any x:Bind statements to the xaml file causes the generation of code that subscribes to the Activated event handler of the window. Using x:Bind in pages added to the window works fine.

Most XAML objects seem to have a mechanism to avoid this issue of circular references (maybe because they derive from DependencyObject?). However, this is not the case for the window object.

Steps to reproduce the bug

See https://github.com/microsoft/microsoft-ui-xaml/issues/7282#issuecomment-2304570650

Expected behavior

Window objects should be able to resolve circular references that occur due to to the registration of event handlers

Screenshots

No response

NuGet package version

WinUI 3 - Windows App SDK 1.6.0: 1.6.240829007

Windows version

Windows 11 (22H2): Build 22621

Additional context

No response

BendicoE commented 2 months ago

Thank you @lhak for this concise report! Hopefully, this can lead to someone in the WinUI 3 - Windows App SDK team to take action and at least give us some feedback.

We are fighting with this bug in our multi-window WinUI 3 application, which is in production since May. And, allthough we have reduced the memory leak on every window close, by manually dereferencing (= null) a number of bindings to potentially large objects, it will simply be impossible to "unwind" everything. We have even refactored our entire MainWindow.xaml to a UserControl, but it seems this doesn't really help as long as this UserControl is contained in a Window derived class.

lhak commented 2 months ago

@BendicoE I personally found the Visual Studio memory snapshots feature quite useful to see what references exist to elements that do not get deallocated.

BendicoE commented 2 months ago

Thanks for the tip @lhak! I have tried the memory snapshots in the profiler in Visual Studio before, and I struggled a bit with it because it would suddenly stop my session quite quickly. I have now started using it with a Release build of our app, and I have made a test function that creates stripped down Window's, without any actual content, only UI. And here is what I found, which I think is extremely interesting: For each new Window created in out app, around 1000 Microsoft.UI.Xaml.Controls.xxxxx objects are created. After closing the window, absolutely none of these are released. If I continue creating and closing windows, these object allocations just accumulate by 1000 per window.

At the moment, I have absolutely no idea how to work around this problem. We are left with only one option: we have to tell our customers to shut down the application after having worked with a few new windows. (After around 20 windows, the application - in any new window - is now so slow that it is unusable and eventually freezes completely.) This is of course totally unacceptable. Needless to say, our situation is pretty desperate.

lhak commented 2 months ago

I did try the sample app from the other thread and put all the UI into a separate UserControl and added that to the window. This still seems to work fine with the ui controls and model being deallocated eventually after the window is closed.

Did you try to see if there are any references to the window or user control object left after the window has been closed? (in my experience you might have to enable "Show dead objects" and disable "Show hot paths only")