iOS PopModalAsync causes memory leaks
Description
Whne you push a modal page that is wrapped in a NavigationPage
Navigation.PushModalAsync(new NavigationPage(new ModularPage())); on iOS we can see that the Page is never released from the memory. We are using Perfiew for debugging.
Here is a GCDump where we open and close a modal page multiple times 20240326_110245_0.zip
I am adding a sample which has both solutions outcommented. Seems counterintuitive that we need to call DisconnectHandler on existing pages and controls when popping a page from the Navigation stack.
Steps to Reproduce
- Open sample app
- Start profiling
- Press the "Click me" button
- Press "Close" button
- Repeat steps 3 and 4
- MAke a memory GCdump
Link to public reproduction project repository
No response
Version with bug
8.0.6 SR1
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
Affected platform versions
No response
Did you find any workaround?
Seems that this can only be fixed by running disconnectHanler on each element on the Page including the page
page.Handler?.DisconnectHandler();
MyLayout.Handler?.DisconnectHandler();
Handler?.DisconnectHandler();
or creating a custom handler for the NavigationPage and running
ViewController.Dispose();
var viewController = ViewController as PageViewController;
viewController.CurrentView = null;
Relevant log output
No response
Possible duplicate or related:
https://github.com/dotnet/maui/issues/20094 https://github.com/dotnet/maui/issues/20119
/similarissues
Hi I'm an AI powered bot that finds similar issues based off the issue title.
Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one and thumbs upping the other issue to help us prioritize it. Thank you!
Open similar issues:
- Page-level memory leak on modal navigation on iOS (#20094), similarity score: 0.84
- Page-level memory leak in NavigationPage on iOS (#20119), similarity score: 0.81
Closed similar issues:
- Page weak reference is still alive after pushing and popping pages modally or normally (#21143), similarity score: 0.78
- Memory Leak on Page Navigation while removing the page. (#21403), similarity score: 0.78
- Navigation.PopModalAsync not working on iOS (#3959), similarity score: 0.78
Note: You can give me feedback by thumbs upping or thumbs downing this comment.
Can you test with the latest nightly build? https://github.com/dotnet/maui/wiki/Nightly-Builds
I just tried with 8.0.60-ci.net8.24303.1 which seems to be the latest on https://dev.azure.com/xamarin/public/_artifacts/feed/maui-nightly
But creating a modal page with a NavigationPage still stops the page from being collected by the garbage collector on a real iOS device. A plain modal with no NavigationPage does get cleaned up after GC Collect.
GC.Collect();
GC.WaitForPendingFinalizers();
See attached test app. To test tap one of the page buttons, then navigate back and tap the Collect GC button. The isAlive should return to false and the text should change from red to black if it has been successfully cleaned up by the garbage collector. The destructors on the BasePage and BaseViewModel classes should also emit a Debug.Writeline() to the console when they are cleaned up.
NavigationPage has circular references, which would be unrelated to modals:
- https://github.com/dotnet/maui/pull/22810
You can just do this to see it:
// The original NavigationPage & children leak
Application.Current.MainPage = new NavigationPage(new Page1());
Application.Current.MainPage = new Page2();
Does the problem still occur with the 8.0.60 service release? The issue with NavigationPage was resolved there.
@jonathanpeppers I have tried the latest 8.0.60 version , however I can see that the memory is still not released. There are a couple problems I can see. First and foremost, NavigationRenderer has a hard reference to the Current Element
public VisualElement Element { get => _viewHandlerWrapper.Element ?? _element; }
This means that the page has a circular reference as the NavigationPage.Handler has a reference to the Renderer. So what we do currently is
public class CustomNavigationHandler : NavigationRenderer
{
protected override void Dispose(bool disposing)
{
base.Dispose(disposing);
SetElement(null);
}
}
Secondly, there is a more common behavior that we can see. Currently, Handlers disposure is not connected to anything. As Microsoft documentation says, the developers "should" decide when to disconnect the handler of each Element. However, because each VisualElement has a handler with a hard reference, the page will not be release until page.Handler?.DisconnectHandler() is called.
My question is, is it still true that this is the way the framework is indented? This would mean that each MAUI dev would need to in one way or another extend the Navigation service and release elements manually. We have tried previously to hook this into Unloaded as specified in docs, but Unloaded is being called when we navigate away from a page, either in its Navigation stack or in a TabbedPage, making Unloaded unreliable event for page disposure.
@era-maui does the problem go away if you use the stock NavigationPage as-is?
After #22810, NavigationPage is able to go away in the test and attached sample app. In your example, what type lives forever?
As for the architectural questions, I'm on the .NET for Android team, so I don't know why some of the decisions were made. I'm just trying to help fix some of these issues. Thanks!
Hi @jonathanpeppers the Memory leak still exists in 8.0.60 on iOS, see attached repo. Tap "Modal with Navigation Page" > Tap Done > Tap/spam Collect GC, the page is never collected.
@jonathanpeppers Understandable. I tested with a regular NavigationPage and the results were the same. I looked at the test from the commit you referenced. Are we sure that CreateHandlerAndAddToWindow is equivalent to await Navigation.PushModalAsyncFixed(new NavigationPage(page)); ? Does it engage the NavigationRenderer the same way?
I'm testing your latest sample today, thanks for sharing. It's possible there is something we missed.
I think this will fix it:
- https://github.com/dotnet/maui/pull/23164
Thanks @jonathanpeppers !