Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[controls] fix memory leaks in
Page
& navigation
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
- Loading branch information