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

[controls] fix memory leaks in Page & navigation #13833

Merged
merged 3 commits into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,16 @@ public override void ViewWillAppear(bool animated)
base.ViewWillAppear(animated);
}

public override void WillMoveToParentViewController(UIViewController parent)
{
base.WillMoveToParentViewController(parent);

foreach (var child in ChildViewControllers)
{
child.WillMoveToParentViewController(parent);
}
}

protected override void Dispose(bool disposing)
{
if (_disposed)
Expand Down
1 change: 1 addition & 0 deletions src/Controls/src/Core/HandlerImpl/Window/Window.Impl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ void OnPageChanged(Page? oldPage, Page? newPage)
{
if (oldPage != null)
{
_menuBarTracker.Target = null;
InternalChildren.Remove(oldPage);
oldPage.HandlerChanged -= OnPageHandlerChanged;
oldPage.HandlerChanging -= OnPageHandlerChanging;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ Task SetupWindowForTests<THandler>(IWindow window, Func<Task> runTests, IMauiCon
}
finally
{
window.Handler.DisconnectHandler();
window.Handler?.DisconnectHandler();
await Task.Delay(10);
newWindow?.Close();
appStub.Handler.DisconnectHandler();
appStub.Handler?.DisconnectHandler();
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ void SetupBuilder()
handlers.AddHandler<Page, PageHandler>();
handlers.AddHandler<Window, WindowHandlerStub>();
handlers.AddHandler<Frame, FrameRenderer>();
handlers.AddHandler<Label, LabelHandler>();
});
});
}
Expand Down Expand Up @@ -287,5 +288,47 @@ await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async
};
});
}

[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!");
}

[Fact(DisplayName = "Can Reuse Pages")]
public async Task CanReusePages()
{
SetupBuilder();
var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });
var reusedPage = new ContentPage
{
Content = new Label()
};

await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async (handler) =>
{
await navPage.Navigation.PushAsync(reusedPage);
await navPage.Navigation.PopAsync();
await navPage.Navigation.PushAsync(reusedPage);
await OnLoadedAsync(reusedPage.Content);
});
}
}
}
58 changes: 57 additions & 1 deletion src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ await CreateHandlerAndAddToWindow<ShellHandler>(shell, async (handler) =>
});
}

#if !IOS
#if !IOS && !MACCATALYST
[Fact]
public async Task ChangingToNewMauiContextDoesntCrash()
{
Expand Down Expand Up @@ -917,6 +917,62 @@ await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async
}
#endif

[Fact(DisplayName = "Pages Do Not Leak")]
public async Task PagesDoNotLeak()
{
SetupBuilder();
var shell = await CreateShellAsync(shell =>
{
shell.CurrentItem = new ContentPage() { Title = "Page 1" };
});

WeakReference pageReference = null;

await CreateHandlerAndAddToWindow<ShellHandler>(shell, async (handler) =>
{
await OnLoadedAsync(shell.CurrentPage);

var page = new ContentPage { Title = "Page 2" };
pageReference = new WeakReference(page);

await shell.Navigation.PushAsync(page);
await shell.Navigation.PopAsync();
});

// 3 GCs were required in Android API 23, 2 worked otherwise
for (int i = 0; i < 3; i++)
{
await Task.Yield();
GC.Collect();
GC.WaitForPendingFinalizers();
}

Assert.NotNull(pageReference);
Assert.False(pageReference.IsAlive, "Page should not be alive!");
}

[Fact(DisplayName = "Can Reuse Pages")]
public async Task CanReusePages()
{
SetupBuilder();
var rootPage = new ContentPage();
var shell = await CreateShellAsync(shell =>
{
shell.CurrentItem = rootPage;
});
var reusedPage = new ContentPage
{
Content = new Label { Text = "HEY" }
};
await CreateHandlerAndAddToWindow<IWindowHandler>(shell, async (handler) =>
{
await shell.Navigation.PushAsync(reusedPage);
await shell.Navigation.PopAsync();
await shell.Navigation.PushAsync(reusedPage);
await OnLoadedAsync(reusedPage.Content);
});
}

protected Task<Shell> CreateShellAsync(Action<Shell> action) =>
InvokeOnMainThreadAsync(() =>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ public static void MapContent(IContentViewHandler handler, IContentView page)
protected override void DisconnectHandler(ContentViewGroup platformView)
{
// If we're being disconnected from the xplat element, then we should no longer be managing its children
platformView.CrossPlatformMeasure = null;
platformView.CrossPlatformArrange = null;
platformView.RemoveAllViews();
base.DisconnectHandler(platformView);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,12 @@ public static void MapContent(IContentViewHandler handler, IContentView page)
{
UpdateContent(handler);
}

protected override void DisconnectHandler(ContentPanel platformView)
{
platformView.CrossPlatformMeasure = null;
platformView.CrossPlatformArrange = null;
base.DisconnectHandler(platformView);
}
}
}
7 changes: 7 additions & 0 deletions src/Core/src/Handlers/ContentView/ContentViewHandler.iOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,12 @@ public static void MapContent(IContentViewHandler handler, IContentView page)
{
UpdateContent(handler);
}

protected override void DisconnectHandler(ContentView platformView)
{
platformView.CrossPlatformMeasure = null;
platformView.CrossPlatformArrange = null;
base.DisconnectHandler(platformView);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ public override void OnResume()
base.OnResume();
}

public override void OnDestroy()
{
_currentView = null;
_fragmentContainerView = null;

base.OnDestroy();
}

public override Animation OnCreateAnimation(int transit, bool enter, int nextAnim)
{
int id = 0;
Expand Down
11 changes: 11 additions & 0 deletions src/Core/src/Platform/iOS/ContainerViewController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,5 +90,16 @@ public override void ViewDidLayoutSubviews()
}

public void Reload() => SetView(CurrentView, true);

public override void WillMoveToParentViewController(UIViewController? parent)
{
if (parent == null)
{
var view = ViewIfLoaded ?? currentPlatformView;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use this to call Disconnect also ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixed caused a different problem, so I'm trying 9a36b51 now.

view?.ClearSubviews();
}

base.WillMoveToParentViewController(parent);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ override Microsoft.Maui.Handlers.EditorHandler.PlatformArrange(Microsoft.Maui.Gr
override Microsoft.Maui.Handlers.RadioButtonHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect frame) -> void
override Microsoft.Maui.Layouts.FlexBasis.Equals(object? obj) -> bool
override Microsoft.Maui.Layouts.FlexBasis.GetHashCode() -> int
override Microsoft.Maui.Platform.NavigationViewFragment.OnDestroy() -> void
override Microsoft.Maui.SizeRequest.Equals(object? obj) -> bool
override Microsoft.Maui.SizeRequest.GetHashCode() -> int
static Microsoft.Maui.FontSize.operator !=(Microsoft.Maui.FontSize left, Microsoft.Maui.FontSize right) -> bool
Expand Down
2 changes: 2 additions & 0 deletions src/Core/src/PublicAPI/net-ios/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ Microsoft.Maui.Hosting.MauiApp.DisposeAsync() -> System.Threading.Tasks.ValueTas
Microsoft.Maui.Layouts.FlexBasis.Equals(Microsoft.Maui.Layouts.FlexBasis other) -> bool
Microsoft.Maui.LifecycleEvents.iOSLifecycle.PerformFetch
Microsoft.Maui.SizeRequest.Equals(Microsoft.Maui.SizeRequest other) -> bool
override Microsoft.Maui.Handlers.ContentViewHandler.DisconnectHandler(Microsoft.Maui.Platform.ContentView! platformView) -> void
override Microsoft.Maui.Handlers.SwipeItemButton.Frame.get -> CoreGraphics.CGRect
override Microsoft.Maui.Handlers.SwipeItemButton.Frame.set -> void
override Microsoft.Maui.Handlers.SwipeItemMenuItemHandler.ConnectHandler(UIKit.UIButton! platformView) -> void
override Microsoft.Maui.Handlers.SwipeItemMenuItemHandler.DisconnectHandler(UIKit.UIButton! platformView) -> void
override Microsoft.Maui.Handlers.SwitchHandler.NeedsContainer.get -> bool
override Microsoft.Maui.Layouts.FlexBasis.Equals(object? obj) -> bool
override Microsoft.Maui.Layouts.FlexBasis.GetHashCode() -> int
override Microsoft.Maui.Platform.ContainerViewController.WillMoveToParentViewController(UIKit.UIViewController? parent) -> void
override Microsoft.Maui.Platform.MauiTextView.Font.get -> UIKit.UIFont?
override Microsoft.Maui.Platform.MauiTextView.Font.set -> void
override Microsoft.Maui.SizeRequest.Equals(object? obj) -> bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ Microsoft.Maui.IApplication.UserAppTheme.get -> Microsoft.Maui.ApplicationModel.
Microsoft.Maui.Hosting.MauiApp.DisposeAsync() -> System.Threading.Tasks.ValueTask
Microsoft.Maui.Layouts.FlexBasis.Equals(Microsoft.Maui.Layouts.FlexBasis other) -> bool
Microsoft.Maui.SizeRequest.Equals(Microsoft.Maui.SizeRequest other) -> bool
override Microsoft.Maui.Handlers.ContentViewHandler.DisconnectHandler(Microsoft.Maui.Platform.ContentView! platformView) -> void
override Microsoft.Maui.Handlers.SwipeItemButton.Frame.get -> CoreGraphics.CGRect
override Microsoft.Maui.Handlers.SwipeItemButton.Frame.set -> void
override Microsoft.Maui.Handlers.SwipeItemMenuItemHandler.ConnectHandler(UIKit.UIButton! platformView) -> void
override Microsoft.Maui.Handlers.SwipeItemMenuItemHandler.DisconnectHandler(UIKit.UIButton! platformView) -> void
override Microsoft.Maui.Handlers.SwitchHandler.NeedsContainer.get -> bool
override Microsoft.Maui.Layouts.FlexBasis.Equals(object? obj) -> bool
override Microsoft.Maui.Layouts.FlexBasis.GetHashCode() -> int
override Microsoft.Maui.Platform.ContainerViewController.WillMoveToParentViewController(UIKit.UIViewController? parent) -> void
override Microsoft.Maui.Platform.MauiTextView.Font.get -> UIKit.UIFont?
override Microsoft.Maui.Platform.MauiTextView.Font.set -> void
override Microsoft.Maui.SizeRequest.Equals(object? obj) -> bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Microsoft.Maui.Hosting.MauiApp.DisposeAsync() -> System.Threading.Tasks.ValueTas
Microsoft.Maui.Layouts.FlexBasis.Equals(Microsoft.Maui.Layouts.FlexBasis other) -> bool
Microsoft.Maui.Platform.MauiWebView.MauiWebView(Microsoft.Maui.Handlers.WebViewHandler! handler) -> void
Microsoft.Maui.SizeRequest.Equals(Microsoft.Maui.SizeRequest other) -> bool
override Microsoft.Maui.Handlers.ContentViewHandler.DisconnectHandler(Microsoft.Maui.Platform.ContentPanel! platformView) -> void
override Microsoft.Maui.Layouts.FlexBasis.Equals(object? obj) -> bool
override Microsoft.Maui.Layouts.FlexBasis.GetHashCode() -> int
override Microsoft.Maui.SizeRequest.Equals(object? obj) -> bool
Expand Down