Widespread memory leak propagation in visual trees

[AdamEssenmacher]

Description

Memory leaks in MAUI are exacerbated by orders of magnitude due to their propagation through the visual tree via held references (e.g. Parent, Content, ItemsSource, Children). This leads to entire pages and binding contexts being held in memory indefinitely (sometimes even entire apps), resulting in severe performance degradation, UI choppiness, and eventual forced application shutdowns by the operating system.

I believe this is the most critically severe performance-related issue in MAUI apps today. The behavior is the same across platforms. Lots of effort has been going into fixing individual leaks, but the ROI on these efforts is muted by the all-or-nothing nature of this problem.

Steps to Reproduce

No response

Link to public reproduction project repository

https://github.com/AdamEssenmacher/MemoryToolkit.Maui/tree/main/samples

Version with bug

8.0.7 SR2

Is this a regression from previous behavior?

Yes, this used to work in Xamarin.Forms

Last version that worked well

Unknown/Other

Affected platforms

iOS, Android, Windows, macOS, I was not able test on other platforms

Affected platform versions

No response

Did you find any workaround?

There are three distinct actions that, when applied systemically through the visual tree bottom-up (from leaf to root), can whack leaky views into non-leaking states and compartmentalize small leaks so that they remain isolated to their offending views:

1) Clear the binding context: This contributes to compartmentalization by removing the reference. It also gives the view a chance to reset itself to a near-default state where leaks are least likely to happen (some views only leak in certain states).
2) Clear references to other views: By clearing properties like Content and ItemsSource and calling ClearLogicalChildren(), we can achieve compartmentalization by breaking the visual tree apart.
3) Disconnect (and, if applicable, dispose) the handler: Many handlers are written in such a way that problematic event handlers are only cleaned up when explicitly disconnected.

View Lifecycle Challenges

The workaround I've offered above might not be an ideal or permanent solution. Executing these cleanup measures requires that we have knowledge of when an app is "done with" a given view or page. MAUI currently does not really have a standardized component lifecycle, so there isn't really a central event or hook we can rely on. I have developed an approach that attempts to address this by making an educated guess about when an app is probably "done with" a given Element by checking for these conditions:

• The Element's Page (or itself, if the Element is a Page) was just popped off the navigation stack.
• The Element has been unloaded and is not (or no longer) hosted within a Page (e.g. a ControlTemplate that was just swapped out).
• The Element is hosted within a NavigationPage that has been unloaded

Stopgap Solution

I have developed a behavior that detects leaky views during development using this definition of "done with", and another behavior that automatically applies the cleanup measures described earlier. I offer them as stop-gap measures pending a more robust solution.

Request for Assistance

I encourage the community to provide feedback on these behaviors, and to contribute ideas for how MAUI might incorporate improved lifecycle management to prevent and contain memory leaks.

Relevant log output

No response

[jonmdev]

Adam, I know you are very knowledgeable on this subject. I have tried to figure this out a bit myself but don't have a shred of your knowledge on it.

For my part, I am creating everything through C# (no XAML). I do personally know with certainty when my objects are not needed, so I can theoretically destroy/dispose of them when needed. But I am still not sure what to do with them as there is no built in "Dispose" system.

"Dispose" Protocol?

I have therefore wanted to create a "Dispose" method that can destroy I think the clearest way I would be able to conceptualize the problem and solution would be. I think this will help others as well or help formulate a correct mechanism for dealing with this.

I think if we had an extension for View "Dispose" that safely discarded that object (and/or its children) then this would be the best way to manage things and be sure we are not having memory leaks.

Example Hierarchy

Let's say hypothetically you have a hierarchy like this:

ContentPage mainPage = new();
this.MainPage = mainPage;

AbsoluteLayout rootToKeep = new();
mainPage.Content = rootToKeep;

AbsoluteLayout rootToDestroy = new();
rootToKeep.Add(rootToDestroy);

Border borderToDestroy = new();
 rootToDestroy.Add(borderToDestroy);

Image imageToDestroy = new();
borderToDestroy.Content = imageToDestroy ;

Label labelToDestroy = new();
rootToDestroy.Add(labelToDestroy);

This creates a hierarchy like this:

ContentPage mainPage
--AbsoluteLayout rootToKeep
-----AbsoluteLayout rootToDestroy
---------Border borderToDestroy 
-------------Image imageToDestroy 
---------Label labelToDestroy

Disposing Correctly?

Hypothetically, let's say you want to destroy the entire rootToDestroy and all its children. Can you step through the correct way to do this in C#, while avoiding memory leaks? Does it differ for the different view types or it is a generic process? Do we need per platform code to dispose the underlying views in their respective systems?

I asked this question here also but got zero replies except someone wondering the same thing as me. Based on the discussions I have seen you participating I suspect you might be the only person that knows and this would again help make clear how we are supposed to manage these things without leaking.

If we have a protocol of steps to follow then we can all apply our own methods that should work as needed.

Thanks for any help if you can.

[AdamEssenmacher]

@jonmdev great questions. You've clearly done a ton of research, but still are still unclear on an ideal approach to ensure MAUI views are cleaned up properly. This underscores my position that MAUI really needs a standard component lifecycle management mechanism.

I'll try to respond to your thoughts to the best of my understanding.

I do personally know with certainty when my objects are not needed, so I can theoretically destroy/dispose of them when needed.

A word of warning: a completely manual approach here is bound to be error-prone, and a single miss can carry a very high penalty. That said, determining 'when' views should be cleaned up is definitely a separate (but somewhat related) concern vs the 'how'. I'll keep the focus on the remainder of my response on the latter.

I think if we had an extension for View "Dispose" that safely discarded that object (and/or its children) then this would be the best way to manage things and be sure we are not having memory leaks.

I think this is an excellent idea! The implementation of AutoDisconnectBehavior I linked in the original issue has (almost) exactly this: https://github.com/AdamEssenmacher/MemoryToolkit.Maui/blob/75656001dda5a6cad29b57cfb3c639f9814ca035/src/MemoryToolkit.Maui/AutoDisconnectBehavior.cs#L130-L206

Currently, the method signature is

private static void Disconnect(IVisualTreeElement vte)

...but there's no reason it couldn't be refactored out and tweaked for more general use like:

public static void TearDown(this IVisualTreeElement vte)

let's say you want to destroy the entire rootToDestroy and all its children. Can you step through the correct way to do this in C#, while avoiding memory leaks?

I'm not going to presume to have a "correct" (or even complete) approach here. This is part of why I've posted the original issue--I can't help but feel that I might be working against MAUI's design intent. However, I can say that the approach I've cobbled together works well in practice 😊.

The poorly-named Disconnect method from my behavior linked above is the core of that approach. Basically it:

1. Works from the bottom-up
2. Clears each element's BindingContext
3. Clears references to other views
4. Disposes the view's handler (if it implements IDisposable) and calls DisconnectHandler()

This approach prevents certain classes of leaks. In particular:

• Leaks that can be avoided by resetting the element to a near-default state
• Leaks that can be avoided by disposing and/or disconnecting the handler

This won't prevent or avoid all leaks though. In these cases, this approach at least compartmentalizes leaks.

I'll also note there there is some 'extra stuff' in the current implementation of Disconnect that shows how this approach can be extended to address platform or app-specific issues:

  if (element.Handler != null)
  {
      OnDisconnectingHandler?.Invoke(null, new DisconnectingHandlerEventArgs(element));

#if IOS
      // Fixes issue specific to ListView on iOS, where RealCell is not nulled out.
      if (element is ViewCell && element.Handler.PlatformView is IDisposable disposablePlatformView)
          disposablePlatformView.Dispose();
#endif
   .....

Extra stuff 1)

OnDisconnectingHandler's purpose here is to provide an event hook for developers to apply targeted clean-up measures that are either not currently handled by the behavior's implementation, or perhaps app-specific. For example, until recently an SKLottieView from the Skia library would leak if a certain property was set to True. Developers could use this event to whack the view back into a non-leaking state by setting the property to false.

Extra stuff 2)

The other part here is targeting a specific, known leak in MAUI's ViewCell that can be avoided if we call Dispose() on the handler's PlatformView. I stress that this is a specific, targeted measure. We should not typically be calling Dispose() on a handler's properties, and in some cases doing so will cause problems.

Does it differ for the different view types or it is a generic process?

The general approach is the same, but accomplishing part (3) (clearing references to other view) can be specific to different type of views. For example:

      if (vte is ListView listView)
          listView.ItemsSource = null;
      else if (vte is ContentView contentView)
          contentView.Content = null;
      else if (vte is Border border)
          border.Content = null;
      else if (vte is ContentPage contentPage)
          contentPage.Content = null;
      else if (vte is ScrollView scrollView)
          scrollView.Content = null;

MAUI's views have something like a common IContentView (or something similar, I don't remember the exact name), for views with a Content property, but it only has a get accessor. Maybe we could use reflection, idk.

Also, this if/else if implementation is likely incomplete.

Do we need per platform code to dispose the underlying views in their respective systems?

Platform-specific leaks exist. Once a leak happens, the leak propagation problem is platform-agnostic, and so the benefit of compartmentalization is as well. Platform handlers really should be taking care of platform specific cleanup themselves. However, as we've seen in the iOS-specific ViewCell example above, this doesn't always happen and then we need to apply a platform-specific workaround.

[jonmdev]

Thanks. That is all very helpful. It is interesting. It feels like these are things we should not need to know about as end users. Ie. There should simply be a function VisualElement.Dispose() that disconnects from the hierarchy the view and its children and does all this you suggest.

What are we SUPPOSED to do?

The question I have next then is what are we intended to do to destroy/dispose things from Microsoft's perspective?

From the reference here: https://learn.microsoft.com/en-us/dotnet/maui/user-interface/handlers/create?view=net-maui-8.0#native-view-cleanup

It sounds like the intended equivalent we are supposed to use is just:

view.Handler?.DisconnectHandler();

Is that supposed to be all we are theoretically supposed to do? If you have something in a hierarchy with children (or without), and you just call this method, what happens and what is supposed to happen?

What about element.CleartLogicalChildren()? or element.Remove() or element.Content = null?

In theory, what does Microsoft/Maui intend for us to be doing here? Are we supposed to manually remove from hierarchy and then DisconnectHandler? Or just DisconnectHandler and in theory everything should happen?

Is this supposed to remove it from the hierarchy as well? Or just dispose of the platform view inside?

Can we have some directions on the intended workflow to remove/destroy/dispose things like Border/Image/Label/Layout?

If we know what we are supposed to be doing we can better comment or work together on how to fix it.

Minor issue?

Also, I am not sure if this will be an issue for you, but your code is:

                    if (element.Handler is IDisposable disposableElementHandler)
                        disposableElementHandler.Dispose();
                    element.Handler.DisconnectHandler();     //WILL THIS HANDLER STILL EXIST? SHOULD IT BE element.Handler?.DisconnectHandler()

I am not sure if you will sometimes get a null Handler there depending on what Dispose does. Perhaps should be element.Handler?.DisconnectHandler() because if Disposed in line above, won't it be null?

[AdamEssenmacher]

What are we SUPPOSED to do?

I think, ideally, we're not supposed to do anything. The GC is supposed to clean things up for us. Handler authors aren't supposed to rely on DisconnectHandler() being called to clean up leaky event subscriptions.

Put another way, I do not believe it is/was in MAUI's design intent for developers to have to manually keep track of views and tear them apart and manually dispose/disconnect them in order to prevent memory leaks. Rather, DisconnectHandler() was probably meant as a tool to allow us to proactively release expensive platform resources (e.g. a video player) when appropriate.

These are just educated guesses though.

Regardless, doing these things is currently necessary (or at least a very good idea) due to this propagating memory leak issue, combined with the practical reality that some handlers do actually rely on DisconnectHandler() to prevent leaky event subscriptions (whether intentional or not).

I am not sure if you will sometimes get a null Handler there depending on what Dispose does. Perhaps should be element.Handler?.DisconnectHandler() because if Disposed in line above, won't it be null?

Generally in C#, calling an instance method like A.B.C() wouldn't change the value of A.B to any other value, null or otherwise. For that to happen, B would need to have a reference to A and the implementation of C would need to use that reference to change the value of A.B--it's theoretically possible, but highly unlikely. The fact that C() in this specific case is actually Dispose() makes no difference. Dispose() is special, but it's not that special.

That aside, you are right to be suspicious of calling methods on a disposed object. Generally in C#, doing so could cause an ObjectDisposedException to be thrown. In this case though, I think I added this Dispose() call specifically in response to a leak in the iOS ListView, and I think calling it before DisconnectHandler was necessary (I might be mistaken though).

[jonmdev]

What are we SUPPOSED to do?

I think, ideally, we're not supposed to do anything. The GC is supposed to clean things up for us. Handler authors aren't supposed to rely on DisconnectHandler() being called to clean up leaky event subscriptions.

What I mean is: Take my example in OP, where we have a hierarchy we are managing by C# and intentionally want to disconnect a chunk and dispose of it. Regardless of any issues around accidental memory leaks, how does Maui intend we do this?

Based on this paragraph here:

Each platform's handler implementation overrides the DisconnectHandler implementation, which is used to perform native view cleanup such as unsubscribing from events and disposing objects. However, this override is intentionally not invoked by .NET MAUI. Instead, you must invoke it yourself from a suitable location in your app's lifecycle. This will often be when the page containing the Video control is navigated away from, which causes the page's Unloaded event to be raised.

I presume the design intention is: (1) manually remove Maui view from Maui hierarchy, and then (2) run DisconnectHandler. That is supposed to lead to the view being garbage collected.

@davidortinau Is this correct? Can you or one of your team please explain what the design intention is? What are we supposed to do in C# if we have a hierarchy and want to remove and garbage collect a view or chunk of that hierarchy?

If so, then @AdamEssenmacher your Disconnect memory leak solutions would have to be rolled into the individual DisconnectHandler overrides to fit the design. For example here.

Whether they would want to add a VisualElement monitor like you do for appropriately triggering this method would be a second issue. ie. There are two problems: (1) proper garbage collection steps when initiated (ie. under DisconnectHandler apparently), and (2) monitoring issues for when it should happen without being requested explicitly (your Monitor solution).

In other words, based on their paragraph above, the fixes you are proposing would have to go into (1) ensuring the Unloaded event is triggered when it should be, and (2) ensuring DisconnectHandler does what it's supposed to when it is called.

[AdamEssenmacher]

I should be extra clear that I'm not proposing any actual fixes here. I've identified a problem in the propagating memory leaks, some major contributing causes, and a workaround.

we have a hierarchy we are managing by C# and intentionally want to disconnect a chunk and dispose of it. Regardless of any issues around accidental memory leaks, how does Maui intend we do this?

Assuming we had a reason to not want to wait around for the GC to take care of it for us, I believe the intent would just be to make sure DisconnectHandler() is called on each view (the root and all of its children) we care to proactively tear down.

I presume the design intention is: (1) manually remove Maui view from Maui hierarchy, and then (2) run DisconnectHandler. That is supposed to lead to the view being garbage collected.

I do not think calling DisconnectHandler() is supposed to have any bearing on whether or not a view should qualify for garbage collection. This would just be an absurd design, especially if developers were expected to actively manage it.

[SittenSpynne]

I keep writing way too much but I think I can answer your question @jonmdev, maybe I won't have to redraft this one.

Put simply: XAML wasn't designed for disposable objects. WPF didn't have that concept. That makes it a bit of a bad fit for MAUI, since under the hood we know native unmanaged objects are correlated with elements.

I think the MAUI team expected the Handler infrastructure would separate things enough it wouldn't matter. What we're finding is it's not enough, or there are some leaks in the internals. They are NOT obvious, so I don't think the MAUI team knows about them. Their impact on smallish apps is limited, so they can go undetected. They only seem to manifest in a catastrophic way in apps intended to be used for a long time with lots of navigations. That's a very "industry or enterprise app" niche on mobile. I think those people have only started using MAUI in earnest this year due to the looming Xamarin Forms support deadline. So now they're finding these issues that could only be found by complicated apps.

I'm super thankful for @AdamEssenmacher. I think he's the authority right here. He's walked backwards through a lot of MAUI code to find the issues. It's hard to overstate how much work that must have been. The memory profiling tools can only tell us "this is the graph" and not "What the heck is that thing and why is it rooting my object?" There really aren't many experts who know that kind of MAUI arcana.

I hope the team sees this and adopts a lot of the code. I have a feeling there are a few patterns they'll find were bad ideas and can be replaced with better ideas. While they hunt that down, I hope they put Microsoft's excellent tech writers in charge of writing a good guide to manually handling these issues and implementing some fixes in apps. I'm curious if MAUI should be a XAML framework with a concept of disposal. As a veteran Windows Forms dev it's never sat right with me that even in WPF elements didn't implement IDisposable just in case.

[PureWeen]

@AdamEssenmacher generally I'd say you've nailed it here with your assessment :-)

in XF we aggressively deconstructed hierarchies and called Dispose on every single platform view. This led to a whole different class of issues that we spent most of the lifetime of XF trying to fix and, in some cases, never did.

• If the unmanaged side of things isn't done with a view (think animations) and we'd disposed of that view on the managed side you would get exceptions from the interop layer trying to reconstruct the view.

  • https://github.com/xamarin/Xamarin.Forms/issues?q=is%3Aissue+is%3Aopen+Unable+to+activate+instance+of+type

• We had somewhat of an opposite problem in XF around causing leaks. Because we depended on dispose to get called on every single renderer, there was a lot of code added to track views in places like ListView and to walk entire hierarchies and make sure we called dispose. I spent a lot of time early on making sure we propagated dispose through every single hierarchy correctly. It's generally a very error prone process.

• It was pretty much impossible in XF to implement a scenario where you'd move a view from one layout to another. Think video player. Because in XF when something was removed from the visual tree we'd dispose the entire hierarchy. We had a number of big issues in XF where people wanted the ability to reuse an xplat view and for it not to get disposed. For example, in XF on Shell it would dispose the entire platform hierarchy when navigating tabs or flyout pages, which made navigation slow all the time. It also wreaked havok on things like MapView.

  • The idea with MAUI was to put the LC in the hands of the developer. So, if you maintain a reference to a View than none of the platform elements would go away. If you don't, then they would.

So, our approach in MAUI was to try to just let the GC do what the GC does and just make sure that all of our code is collectable.

Class of issues we've been fixing and users have hit

Making Handlers GC friendly

This is largely an iOS problem (though it does happen on other platforms). You can very easily cause memory leaks on iOS when there's a circular dependency of NSObjects. This happens for us because all of our XPLAT elements have a "Parent" property, so, all of our children have references to all of our parents.

Example PR: #18682

On Android and Windows this usually comes up with the classic scenario of referencing things that survive the lifetime of what you want GC'd. We don't really have that many PRs for android/windows specific leaks because it doesn't suffer from the same circular reference issue. Typically, with android/windows you don't really even need to unsubscribe from events.
#18584
#16045

Managed Xplat mistakes

There have been a few places where things were getting parented when they shouldn't have
#13656

IDisposable confusion and DI

• If you implement IDisposable and resolve services as transient those types will never GC. I would really like to add some additional features to enable this better with service scopes. https://learn.microsoft.com/en-us/dotnet/core/extensions/dependency-injection-guidelines#idisposable-guidance-for-transient-and-shared-instances

What now?

• I think we're somewhat still in the "Death by a thousand cuts" phase right now. We're working through all the leak scenarios we can and actively putting them in all SRs. Adding any additional automated deconstruction at this point is premature.

• Generally, if you're developing a .NET MAUI app, and you aren't building your own custom handlers, then you shouldn't really need that much information to prevent leaks beyond the general knowledge of what causes things to leak in managed code. If you have a view that's subscribing to a global event, then you just unsubscribe from it inside Unloaded or Disappearing end of story. Loaded/Unloaded are tied directly to the platform indicating that a view has been removed from the visual hierarchy. So, you can use these events on any view inside the entire MAUI ecosystem to cleanup whatever you have that might cause a leak.

• If you're developing handlers, then I would just recommend using the natural lifecycle events of each platform.

  • windows : Loaded/unloaded
  • Android : DetachedFromWindow
  • iOS: willMoveToWindow

At one point during NET8 development we did think about adding some hooks in handler that would just be directly tied to when a view is removed/added from the Visual tree #16292 but deemed that it would be premature to commit to this API.

• If you have scenarios that are still leaking then please log an issue with a repro and we will fix it. Fixing memory leaks has been priority one for NET8 and will continue to be priority 1 until morale improves :-)