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

Every MAUI Page generates a Memory Leak on iOS and macCatalyst #13520

Closed
danardelean opened this issue Feb 23, 2023 · 15 comments · Fixed by #13833 or #14108
Closed

Every MAUI Page generates a Memory Leak on iOS and macCatalyst #13520

danardelean opened this issue Feb 23, 2023 · 15 comments · Fixed by #13833 or #14108
Assignees
Labels
fixed-in-8.0.0-preview.3.8149 Look for this fix in 8.0.0-preview.3.8149! legacy-area-perf Startup / Runtime performance p/1 Work that is critical for the release, but we could probably ship without partner/cat 😻 Client CAT Team platform/iOS 🍎 t/bug Something isn't working t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.)

Comments

@danardelean
Copy link

Description

On iOS and maccatalyst the finalizers are never called and I cannot understand if the page leaks or not.

I know that we use the interpretor (don't know if this could be the problem) but I would expect that at some point the instances that I have in memory should be cleaned.

Btw: How do we profile memory allocations in MAUI on iOS and maccatalyst? Xamarin. Profiler does not work for the MAUI applications.
I looked here:
https://github.com/dotnet/maui/wiki/Profiling-.NET-MAUI-Apps
but what I saw is how you profile startup not the memory allocations.

Any guidance on how to profile memory allocations on iOS, maccatalyst and Android?

Steps to Reproduce

1.Create a new MAUI project
2.Add a SecondPage and register with AppShell
3. For debugging add the finalizer on the SecondPage : ~SecondPage()
4. On MainPage add an toolbar item where you manually call GC.Collect()

Navigating back an forth and pressing the button that does garbage collector the finalizer is never called on iOS and maccatalyst (on Android the Finalizer it is called) so I don't understand if the page leak or not.

Link to public reproduction project repository

https://github.com/danardelean/MauiTestFinalizer

Version with bug

7.0 (current)

Last version that worked well

Unknown/Other

Affected platforms

iOS, macOS

Affected platform versions

iOS 16.3.1, macOS Ventura 13.2.1

Did you find any workaround?

NO

Relevant log output

No response

@danardelean danardelean added the t/bug Something isn't working label Feb 23, 2023
@jsuarezruiz jsuarezruiz added platform/iOS 🍎 legacy-area-perf Startup / Runtime performance labels Feb 23, 2023
@PureWeen
Copy link
Member

@jonathanpeppers FYI

@PureWeen PureWeen added this to the Backlog milestone Feb 23, 2023
@ghost
Copy link

ghost commented Feb 23, 2023

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@jonathanpeppers
Copy link
Member

Btw: How do we profile memory allocations in MAUI on iOS and maccatalyst? Xamarin. Profiler does not work for the MAUI applications.

I wrote a thing to be able to do this on Android:

https://github.com/jonathanpeppers/Mono.Profiler.Android

I built libmono-profiler-log.so from dotnet/runtime, which is not shipped or redistributed anywhere. It has the same old hooks into the Mono runtime the Xamarin profiler used.

Right now, this is currently the only way for apps running on Mono to get memory information. They are investigating if this is something that can be added to dotnet-trace and dotnet-dsrouter in the future.

@danardelean
Copy link
Author

@jonathanpeppers do you know if it can be done for iOS also? Thanks

@danardelean
Copy link
Author

@PureWeen I saw that you added to the Backlog (I still have an issue opened in June 2022 in the Backlog). If this is really a leak it is really big one because it means that every page leaks in iOS and Maccatalyst (I encountered this while trying to debug a problem for one of my clients and it has some really big pages and after a while I receive the applicationDidReceiveMemoryWarning in AppDelegate and then the application crashes).

I also have modified the sample and tried it in release mode and I have the same issues. Windows and Android don't have the issue as they GC's SecondPage

@danardelean danardelean changed the title Possible Memory Leak on iOS and macCatalyst Possible Memory Leak on any Page in iOS and macCatalyst Feb 24, 2023
@danardelean danardelean changed the title Possible Memory Leak on any Page in iOS and macCatalyst Every Page generated a Memory Leak on iOS and macCatalyst Feb 24, 2023
@danardelean danardelean changed the title Every Page generated a Memory Leak on iOS and macCatalyst Every MAUI Page generates a Memory Leak on iOS and macCatalyst Feb 24, 2023
@jonathanpeppers
Copy link
Member

@danardelean you might try Instruments, but it will likely only tell you if memory is growing and not what objects.

I saw an interesting sample here:

https://github.com/ivan-todorov-progress/maui-collection-view-memory-leak-bug

You can try their attached property MemoryTracker.IsTracked="True" to find out if an object is leaking.

@jonathanpeppers
Copy link
Member

@danardelean you should review my recent PRs and the tests I'm adding, lol.

@mikeparker104 mikeparker104 added the partner/cat 😻 Client CAT Team label Feb 28, 2023
@samhouts samhouts added the p/1 Work that is critical for the release, but we could probably ship without label Mar 2, 2023
@mattleibow
Copy link
Member

@rolfbjarne does the debugger "work" with breakpoints in the finalizer? Maybe that is just being collected and the debugger is not being notified/break/something?

@jonathanpeppers jonathanpeppers self-assigned this Mar 6, 2023
@rolfbjarne
Copy link
Member

@rolfbjarne does the debugger "work" with breakpoints in the finalizer? Maybe that is just being collected and the debugger is not being notified/break/something?

The debugger should work in finalizers, but I'm untrusting of debuggers by nature, and would use some other method to report if objects are collected (Console.WriteLine tends to work well).

@danardelean
Copy link
Author

@rolfbjarne If you look at the updated sample I am using a label with the number of instances still in memory. Just to be sure this is not an error of the interpreter on iOS and macOS I ran the sample in release and the instances are still not freed so , it would seem, that the memory leak is real

@jonathanpeppers
Copy link
Member

jonathanpeppers commented Mar 7, 2023

In the sample above, I just changed:

private async void OnCounterClicked(object sender, EventArgs e)
{
    GC.Collect();
    GC.WaitForPendingFinalizers();
    await Shell.Current.GoToAsync($"/{nameof(SecondPage)}");
}

And I consistently only see 1 or 2 pages alive on Windows and Android.

iOS and MacCatalyst the number increases by one each time you navigate, which would indicate Page's leaking.

The page is really simple:

<ContentPage...>
    <VerticalStackLayout>
        <Label
            x:Name="lblInstances"
            Text="Welcome to .NET MAUI!"
            VerticalOptions="Center" 
            HorizontalOptions="Center" />
    </VerticalStackLayout>
</ContentPage>

So this must be related to all iOS/MacCatalyst pages, or perhaps shell navigation?

@jonathanpeppers
Copy link
Member

I can also reproduce it if I remove the Shell Route, and just do Navigation.PushAsync(new SecondPage());.

So this must be something that happens for all Page's.

jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 10, 2023
Fixes: dotnet#13520
Context: https://github.com/danardelean/MauiTestFinalizer

In the above sample, you can see `Page` objects leaking while using
Shell navigation. I was able to reproduce this in a test with both
Shell & `NavigationPage` such as:

    [Fact(DisplayName = "NavigationPage Does Not Leak")]
    public async Task DoesNotLeak()
    {
        SetupBuilder();
        WeakReference pageReference = null;
        var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });

        await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async (handler) =>
        {
            var page = new ContentPage { Title = "Page 2" };
            pageReference = new WeakReference(page);
            await navPage.Navigation.PushAsync(page);
            await navPage.Navigation.PopAsync();
        });

        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Assert.NotNull(pageReference);
        Assert.False(pageReference.IsAlive, "Page should not be alive!");
    }

After a lot of investigating, I found even a "basic" `Page` test leaks:

    [Fact("Basic Page Does Not Leak")]
    public async Task BasicPageDoesNotLeak()
    {
        var pageReference = new WeakReference(new ContentPage());
        await CreateHandlerAndAddToWindow<PageHandler>((Page)pageReference.Target, _ => Task.CompletedTask);
        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Assert.False(pageReference.IsAlive, "Page should not be alive!");
    }

To summarize:

* Customer's sample only leaks on iOS/Catalyst
* My Shell/NavigationPage tests leak on Android/iOS/Catalyst
* My "basic page" test leaks on all platforms.

Comparing various GC heap dumps in Visual Studio, I was able to find
the sources of these issues. I'll try to list them.

* Tests
    * `CreateHandlerAndAddToWindow` should unset `Window.Page`
    * `ControlsHandlerTestBase.Windows.cs` needed some `null` checks
* Controls.Core
    * `ModalNavigationManager` needs a `Disconnect()` method to unset
      `_previousPage` and call `NavigationModel.Clear()`
    * `Window.Destroy()` then needs to call
      `ModalNavigationManager.Disconnect()`
    * `Window.OnPageChanged()` should unset `_menuBarTracker.Target`
* Windows
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
* Android
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
    * `ShellContentFragment.Destroy()` should unset `_page`
    * `NavigationViewFragment.OnDestroy()` should unset `_currentView`
      and `_fragmentContainerView`
    * `ScopedFragment.OnDestroy()` should unset `DetailView`
* iOS/Catalyst
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.

After these changes, my tests pass! They literally would not pass
without fixing all of the above... Each issue I found while reviewing
GC dumps.

WIP:

* I still need to test on iOS/Catalyst & retest user sample
* I need to document how to get `*.gcdump` files w/ Mono runtime
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 14, 2023
Fixes: dotnet#13520
Context: https://github.com/danardelean/MauiTestFinalizer

In the above sample, you can see `Page` objects leaking while using
Shell navigation. I was able to reproduce this in a test with both
Shell & `NavigationPage` such as:

    [Fact(DisplayName = "NavigationPage Does Not Leak")]
    public async Task DoesNotLeak()
    {
        SetupBuilder();
        WeakReference pageReference = null;
        var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });

        await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async (handler) =>
        {
            var page = new ContentPage { Title = "Page 2" };
            pageReference = new WeakReference(page);
            await navPage.Navigation.PushAsync(page);
            await navPage.Navigation.PopAsync();
        });

        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Assert.NotNull(pageReference);
        Assert.False(pageReference.IsAlive, "Page should not be alive!");
    }

After a lot of investigating, I found even a "basic" `Page` test leaks:

    [Fact("Basic Page Does Not Leak")]
    public async Task BasicPageDoesNotLeak()
    {
        var pageReference = new WeakReference(new ContentPage());
        await CreateHandlerAndAddToWindow<PageHandler>((Page)pageReference.Target, _ => Task.CompletedTask);
        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Assert.False(pageReference.IsAlive, "Page should not be alive!");
    }

To summarize:

* Customer's sample only leaks on iOS/Catalyst
* My Shell/NavigationPage tests leak on Android/iOS/Catalyst
* My "basic page" test leaks on all platforms.

Comparing various GC heap dumps in Visual Studio, I was able to find
the sources of these issues. I'll try to list them.

* Tests
    * `CreateHandlerAndAddToWindow` should unset `Window.Page`
    * `ControlsHandlerTestBase.Windows.cs` needed some `null` checks
* Controls.Core
    * `ModalNavigationManager` needs a `Disconnect()` method to unset
      `_previousPage` and call `NavigationModel.Clear()`
    * `Window.Destroy()` then needs to call
      `ModalNavigationManager.Disconnect()`
    * `Window.OnPageChanged()` should unset `_menuBarTracker.Target`
* Windows
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
* Android
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
    * `ShellContentFragment.Destroy()` should unset `_page`
    * `NavigationViewFragment.OnDestroy()` should unset `_currentView`
      and `_fragmentContainerView`
    * `ScopedFragment.OnDestroy()` should unset `DetailView`
* iOS/Catalyst
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.

After these changes, my tests pass! They literally would not pass
without fixing all of the above... Each issue I found while reviewing
GC dumps.

WIP:

* I still need to test on iOS/Catalyst & retest user sample
* I need to document how to get `*.gcdump` files w/ Mono runtime
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 14, 2023
Fixes: dotnet#13520
Context: https://github.com/danardelean/MauiTestFinalizer

In the above sample, you can see `Page` objects leaking while using
Shell navigation. I was able to reproduce this in a test with both
Shell & `NavigationPage` such as:

    [Fact(DisplayName = "NavigationPage Does Not Leak")]
    public async Task DoesNotLeak()
    {
        SetupBuilder();
        WeakReference pageReference = null;
        var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });

        await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async (handler) =>
        {
            var page = new ContentPage { Title = "Page 2" };
            pageReference = new WeakReference(page);
            await navPage.Navigation.PushAsync(page);
            await navPage.Navigation.PopAsync();
        });

        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Assert.NotNull(pageReference);
        Assert.False(pageReference.IsAlive, "Page should not be alive!");
    }

After a lot of investigating, I found even a "basic" `Page` test leaks:

    [Fact("Basic Page Does Not Leak")]
    public async Task BasicPageDoesNotLeak()
    {
        var pageReference = new WeakReference(new ContentPage());
        await CreateHandlerAndAddToWindow<PageHandler>((Page)pageReference.Target, _ => Task.CompletedTask);
        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Assert.False(pageReference.IsAlive, "Page should not be alive!");
    }

To summarize:

* Customer's sample only leaks on iOS/Catalyst
* My Shell/NavigationPage tests leak on Android/iOS/Catalyst
* My "basic page" test leaks on all platforms.

Comparing various GC heap dumps in Visual Studio, I was able to find
the sources of these issues. I'll try to list them.

* Tests
    * `CreateHandlerAndAddToWindow` should unset `Window.Page`
    * `ControlsHandlerTestBase.Windows.cs` needed some `null` checks
* Controls.Core
    * `ModalNavigationManager` needs a `Disconnect()` method to unset
      `_previousPage` and call `NavigationModel.Clear()`
    * `Window.Destroy()` then needs to call
      `ModalNavigationManager.Disconnect()`
    * `Window.OnPageChanged()` should unset `_menuBarTracker.Target`
* Windows
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
* Android
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
    * `ShellContentFragment.Destroy()` should unset `_page`
    * `NavigationViewFragment.OnDestroy()` should unset `_currentView`
      and `_fragmentContainerView`
    * `ScopedFragment.OnDestroy()` should unset `DetailView`
* iOS/Catalyst
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.

After these changes, my tests pass! They literally would not pass
without fixing all of the above... Each issue I found while reviewing
GC dumps.

WIP:

* I still need to test on iOS/Catalyst & retest user sample
* I need to document how to get `*.gcdump` files w/ Mono runtime
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 14, 2023
Fixes: dotnet#13520
Context: https://github.com/danardelean/MauiTestFinalizer

In the above sample, you can see `Page` objects leaking while using
Shell navigation. I was able to reproduce this in a test with both
Shell & `NavigationPage` such as:

    [Fact(DisplayName = "NavigationPage Does Not Leak")]
    public async Task DoesNotLeak()
    {
        SetupBuilder();
        WeakReference pageReference = null;
        var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });

        await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async (handler) =>
        {
            var page = new ContentPage { Title = "Page 2" };
            pageReference = new WeakReference(page);
            await navPage.Navigation.PushAsync(page);
            await navPage.Navigation.PopAsync();
        });

        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Assert.NotNull(pageReference);
        Assert.False(pageReference.IsAlive, "Page should not be alive!");
    }

After a lot of investigating, I found even a "basic" `Page` test leaks:

    [Fact("Basic Page Does Not Leak")]
    public async Task BasicPageDoesNotLeak()
    {
        var pageReference = new WeakReference(new ContentPage());
        await CreateHandlerAndAddToWindow<PageHandler>((Page)pageReference.Target, _ => Task.CompletedTask);
        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Assert.False(pageReference.IsAlive, "Page should not be alive!");
    }

To summarize:

* Customer's sample only leaks on iOS/Catalyst
* My Shell/NavigationPage tests leak on Android/iOS/Catalyst
* My "basic page" test leaks on all platforms.

Comparing various GC heap dumps in Visual Studio, I was able to find
the sources of these issues. I'll try to list them.

* Tests
    * `CreateHandlerAndAddToWindow` should unset `Window.Page`
    * `ControlsHandlerTestBase.Windows.cs` needed some `null` checks
* Controls.Core
    * `ModalNavigationManager` needs a `Disconnect()` method to unset
      `_previousPage` and call `NavigationModel.Clear()`
    * `Window.Destroy()` then needs to call
      `ModalNavigationManager.Disconnect()`
    * `Window.OnPageChanged()` should unset `_menuBarTracker.Target`
* Windows
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
* Android
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
    * `ShellContentFragment.Destroy()` should unset `_page`
    * `NavigationViewFragment.OnDestroy()` should unset `_currentView`
      and `_fragmentContainerView`
    * `ScopedFragment.OnDestroy()` should unset `DetailView`
* iOS/Catalyst
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.

After these changes, my tests pass! They literally would not pass
without fixing all of the above... Each issue I found while reviewing
GC dumps.

WIP:

* I still need to test on iOS/Catalyst & retest user sample
* I need to document how to get `*.gcdump` files w/ Mono runtime
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 15, 2023
Fixes: dotnet#13520
Context: https://github.com/danardelean/MauiTestFinalizer

In the above sample, you can see `Page` objects leaking while using
Shell navigation. I was able to reproduce this in a test with both
Shell & `NavigationPage` such as:

    [Fact(DisplayName = "NavigationPage Does Not Leak")]
    public async Task DoesNotLeak()
    {
        SetupBuilder();
        WeakReference pageReference = null;
        var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });

        await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async (handler) =>
        {
            var page = new ContentPage { Title = "Page 2" };
            pageReference = new WeakReference(page);
            await navPage.Navigation.PushAsync(page);
            await navPage.Navigation.PopAsync();
        });

        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Assert.NotNull(pageReference);
        Assert.False(pageReference.IsAlive, "Page should not be alive!");
    }

To summarize:

* Customer's sample only leaks on iOS/Catalyst

* My Shell/NavigationPage tests leak on Android/iOS/Catalyst

Comparing various GC heap dumps in Visual Studio, I was able to find
the sources of these issues. I'll try to list them.

* Controls.Core
    * `Window.OnPageChanged()` should unset `_menuBarTracker.Target`
* Windows
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
* Android
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
    * `NavigationViewFragment.OnDestroy()` should unset `_currentView`
      and `_fragmentContainerView`
* iOS/Catalyst
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
    * `ContentViewHandler.DisconnectHandler` should call
      `RemoveFromSuperview()`

After these changes, my tests pass! They literally would not pass
without fixing all of the above... Each issue I found while reviewing
GC dumps.

Other test changes:

* `ControlsHandlerTestBase.Windows.cs` needed some `null` checks
* `ShellTests` needed an `&& !MACCATALYST` to build for MacCatalyst
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 15, 2023
Fixes: dotnet#13520
Context: https://github.com/danardelean/MauiTestFinalizer

In the above sample, you can see `Page` objects leaking while using
Shell navigation. I was able to reproduce this in a test with both
Shell & `NavigationPage` such as:

    [Fact(DisplayName = "NavigationPage Does Not Leak")]
    public async Task DoesNotLeak()
    {
        SetupBuilder();
        WeakReference pageReference = null;
        var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });

        await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async (handler) =>
        {
            var page = new ContentPage { Title = "Page 2" };
            pageReference = new WeakReference(page);
            await navPage.Navigation.PushAsync(page);
            await navPage.Navigation.PopAsync();
        });

        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Assert.NotNull(pageReference);
        Assert.False(pageReference.IsAlive, "Page should not be alive!");
    }

To summarize:

* Customer's sample only leaks on iOS/Catalyst

* My Shell/NavigationPage tests leak on Android/iOS/Catalyst

Comparing various GC heap dumps in Visual Studio, I was able to find
the sources of these issues. I'll try to list them.

* Controls.Core
    * `Window.OnPageChanged()` should unset `_menuBarTracker.Target`
* Windows
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
* Android
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
    * `NavigationViewFragment.OnDestroy()` should unset `_currentView`
      and `_fragmentContainerView`
* iOS/Catalyst
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
    * `ContainerViewController`0 should call `ClearSubviews()` on its
      view in `WillMoveToParentViewController()`
    * `ParentingViewController` should forward `WillMoveToParentViewController()`
      to its child view controllers

After these changes, my tests pass! They literally would not pass
without fixing all of the above... Each issue I found while reviewing
GC dumps.

Other test changes:

* `ControlsHandlerTestBase.Windows.cs` needed some `null` checks
* `ShellTests` needed an `&& !MACCATALYST` to build for MacCatalyst
* Added tests proving we can reuse pages still.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 16, 2023
Fixes: dotnet#13520
Context: https://github.com/danardelean/MauiTestFinalizer

In the above sample, you can see `Page` objects leaking while using
Shell navigation. I was able to reproduce this in a test with both
Shell & `NavigationPage` such as:

    [Fact(DisplayName = "NavigationPage Does Not Leak")]
    public async Task DoesNotLeak()
    {
        SetupBuilder();
        WeakReference pageReference = null;
        var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });

        await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async (handler) =>
        {
            var page = new ContentPage { Title = "Page 2" };
            pageReference = new WeakReference(page);
            await navPage.Navigation.PushAsync(page);
            await navPage.Navigation.PopAsync();
        });

        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Assert.NotNull(pageReference);
        Assert.False(pageReference.IsAlive, "Page should not be alive!");
    }

To summarize:

* Customer's sample only leaks on iOS/Catalyst

* My Shell/NavigationPage tests leak on Android/iOS/Catalyst

Comparing various GC heap dumps in Visual Studio, I was able to find
the sources of these issues. I'll try to list them.

* Controls.Core
    * `Window.OnPageChanged()` should unset `_menuBarTracker.Target`
* Windows
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
* Android
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
    * `NavigationViewFragment.OnDestroy()` should unset `_currentView`
      and `_fragmentContainerView`
* iOS/Catalyst
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
    * `ContainerViewController`0 should call `ClearSubviews()` on its
      view in `WillMoveToParentViewController()`
    * `ParentingViewController` should forward `WillMoveToParentViewController()`
      to its child view controllers

After these changes, my tests pass! They literally would not pass
without fixing all of the above... Each issue I found while reviewing
GC dumps.

Other test changes:

* `ControlsHandlerTestBase.Windows.cs` needed some `null` checks
* `ShellTests` needed an `&& !MACCATALYST` to build for MacCatalyst
* Added tests proving we can reuse pages still.
PureWeen pushed a commit that referenced this issue Mar 20, 2023
* [controls] fix memory leaks in `Page` & navigation

Fixes: #13520
Context: https://github.com/danardelean/MauiTestFinalizer

In the above sample, you can see `Page` objects leaking while using
Shell navigation. I was able to reproduce this in a test with both
Shell & `NavigationPage` such as:

    [Fact(DisplayName = "NavigationPage Does Not Leak")]
    public async Task DoesNotLeak()
    {
        SetupBuilder();
        WeakReference pageReference = null;
        var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });

        await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async (handler) =>
        {
            var page = new ContentPage { Title = "Page 2" };
            pageReference = new WeakReference(page);
            await navPage.Navigation.PushAsync(page);
            await navPage.Navigation.PopAsync();
        });

        await Task.Yield();
        GC.Collect();
        GC.WaitForPendingFinalizers();
        Assert.NotNull(pageReference);
        Assert.False(pageReference.IsAlive, "Page should not be alive!");
    }

To summarize:

* Customer's sample only leaks on iOS/Catalyst

* My Shell/NavigationPage tests leak on Android/iOS/Catalyst

Comparing various GC heap dumps in Visual Studio, I was able to find
the sources of these issues. I'll try to list them.

* Controls.Core
    * `Window.OnPageChanged()` should unset `_menuBarTracker.Target`
* Windows
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
* Android
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
    * `NavigationViewFragment.OnDestroy()` should unset `_currentView`
      and `_fragmentContainerView`
* iOS/Catalyst
    * `ContentViewHandler.DisconnectHandler` should unset
      `platformView.CrossPlatformMeasure` and
      `platformView.CrossPlatformArrange`.
    * `ContainerViewController`0 should call `ClearSubviews()` on its
      view in `WillMoveToParentViewController()`
    * `ParentingViewController` should forward `WillMoveToParentViewController()`
      to its child view controllers

After these changes, my tests pass! They literally would not pass
without fixing all of the above... Each issue I found while reviewing
GC dumps.

Other test changes:

* `ControlsHandlerTestBase.Windows.cs` needed some `null` checks
* `ShellTests` needed an `&& !MACCATALYST` to build for MacCatalyst
* Added tests proving we can reuse pages still.

* [ios] go back to `platformView.RemoveFromSuperview()`
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 21, 2023
Further fixes: dotnet#13520

7d0af63 did not appear to be a complete fix for dotnet#13520, as when I
retested the customer sample, I found I needed to remove a
`VerticalStackLayout` for the problem to go away.

I could reproduce the issue in a test by doing:

    var page = new ContentPage { Title = "Page 2", Content = new VerticalStackLayout { new Label() } };

After lots of debugging, I found the underlying issue was that
`MauiView` on iOS held a reference to the cross-platform `IView`.

After changing the backing field for this to be a `WeakReference<IView>`
the above problem appears to be solved.

I also removed `LayoutView.CrossplatformArrange/Measure` as we can just
use the `View` property directly instead.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 23, 2023
Further fixes: dotnet#13520

7d0af63 did not appear to be a complete fix for dotnet#13520, as when I
retested the customer sample, I found I needed to remove a
`VerticalStackLayout` for the problem to go away.

I could reproduce the issue in a test by doing:

    var page = new ContentPage { Title = "Page 2", Content = new VerticalStackLayout { new Label() } };

After lots of debugging, I found the underlying issue was that
`MauiView` on iOS held a reference to the cross-platform `IView`.

After changing the backing field for this to be a `WeakReference<IView>`
the above problem appears to be solved.

I also removed `LayoutView.CrossplatformArrange/Measure` as we can just
use the `View` property directly instead.
PureWeen pushed a commit that referenced this issue Mar 29, 2023
Further fixes: #13520

7d0af63 did not appear to be a complete fix for #13520, as when I
retested the customer sample, I found I needed to remove a
`VerticalStackLayout` for the problem to go away.

I could reproduce the issue in a test by doing:

    var page = new ContentPage { Title = "Page 2", Content = new VerticalStackLayout { new Label() } };

After lots of debugging, I found the underlying issue was that
`MauiView` on iOS held a reference to the cross-platform `IView`.

After changing the backing field for this to be a `WeakReference<IView>`
the above problem appears to be solved.

I also removed `LayoutView.CrossplatformArrange/Measure` as we can just
use the `View` property directly instead.
@samhouts samhouts added the fixed-in-8.0.0-preview.3.8149 Look for this fix in 8.0.0-preview.3.8149! label Apr 12, 2023
@TranTrieuLamQuynh
Copy link

@danardelean you might try Instruments, but it will likely only tell you if memory is growing and not what objects.

I saw an interesting sample here:

https://github.com/ivan-todorov-progress/maui-collection-view-memory-leak-bug

You can try their attached property MemoryTracker.IsTracked="True" to find out if an object is leaking.

I updated 8.0.0-preview.3.8149. I using BindableLayout in StackLayout and add MemoryTracker.IsTracked="True" to check object leaking. I check by going to this page and back many times. As a result, on iOS the alive object always increments until it crashes.
I also tried it on Xamarin and the alive object decreased after a few tries.

@ghost
Copy link

ghost commented May 8, 2023

Hello lovely human, thank you for your comment on this issue. Because this issue has been closed for a period of time, please strongly consider opening a new issue linking to this issue instead to ensure better visibility of your comment. Thank you!

@samhouts samhouts modified the milestones: Backlog, .NET 8 May 24, 2023
@dotnet dotnet locked as resolved and limited conversation to collaborators Jun 24, 2023
@Eilon Eilon added the t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) label May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixed-in-8.0.0-preview.3.8149 Look for this fix in 8.0.0-preview.3.8149! legacy-area-perf Startup / Runtime performance p/1 Work that is critical for the release, but we could probably ship without partner/cat 😻 Client CAT Team platform/iOS 🍎 t/bug Something isn't working t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.)
Projects
None yet
10 participants