forked from dotnet/maui
-
Notifications
You must be signed in to change notification settings - Fork 1
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 (dotnet#13833)
* [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!"); } 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()`
- Loading branch information
1 parent
d6adf5c
commit 7d0af63
Showing
12 changed files
with
132 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters