Skip to content

Commit

Permalink
[ios/catalyst] fix memory leak in modal pages
Browse files Browse the repository at this point in the history
Fixes: dotnet#20094
Context: https://github.com/AdamEssenmacher/iOSModalLeak.Maui

In the above sample, you can see that modal `Page`'s on iOS or Catalyst
live forever after they are dismissed. I was able to reproduce this
issue in a device test.

After some investigation, the `ContainerViewController` appears to have
a cycle:

* `ContainerViewController` -> `IElement? _view;` -> `PageHandler` -> `ContainerViewController

After 7d0af63 was merged, this works fine when using `NavigationPage`,
but not when using modals.

It appears after solving the cycle, the `ContainerViewController` goes
away as well as the `PageHandler` and the `Page`.
  • Loading branch information
jonathanpeppers committed May 6, 2024
1 parent 222ca3c commit ec3d854
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 11 deletions.
27 changes: 27 additions & 0 deletions src/Controls/tests/DeviceTests/Memory/MemoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,33 @@ void SetupBuilder()
});
}

[Fact("Page Does Not Leak")]
public async Task PageDoesNotLeak()
{
SetupBuilder();

WeakReference viewReference = null;
WeakReference handlerReference = null;
WeakReference platformViewReference = null;

var navPage = new NavigationPage(new ContentPage { Title = "Page 1" });

await CreateHandlerAndAddToWindow(new Window(navPage), async () =>
{
var page = new ContentPage { Content = new Label() };
await navPage.Navigation.PushModalAsync(page);
viewReference = new WeakReference(page);
handlerReference = new WeakReference(page.Handler);
platformViewReference = new WeakReference(page.Handler.PlatformView);
await navPage.Navigation.PopModalAsync();
});

await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
}

[Theory("Handler Does Not Leak")]
[InlineData(typeof(ActivityIndicator))]
[InlineData(typeof(Border))]
Expand Down
21 changes: 10 additions & 11 deletions src/Core/src/Platform/iOS/ContainerViewController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ namespace Microsoft.Maui.Platform
{
public class ContainerViewController : UIViewController, IReloadHandler
{
[UnconditionalSuppressMessage("Memory", "MEM0002", Justification = "Proven safe in test: NavigationPageTests.DoesNotLeak")]
IElement? _view;
WeakReference<IElement>? _view;
[UnconditionalSuppressMessage("Memory", "MEM0002", Justification = "Proven safe in test: NavigationPageTests.DoesNotLeak")]
UIView? currentPlatformView;

Expand All @@ -21,7 +20,7 @@ public class ContainerViewController : UIViewController, IReloadHandler

public IElement? CurrentView
{
get => _view;
get => _view?.GetTargetOrDefault();
set => SetView(value);
}

Expand All @@ -33,15 +32,15 @@ public class ContainerViewController : UIViewController, IReloadHandler

void SetView(IElement? view, bool forceRefresh = false)
{
if (view == _view && !forceRefresh)
if (_view?.GetTargetOrDefault() is IElement existingView && view == existingView && !forceRefresh)
return;

_view = view;
_view = view is null ? null : new(view);

if (view is ITitledElement page)
Title = page.Title;

if (_view is IHotReloadableView ihr)
if (view is IHotReloadableView ihr)
{
ihr.ReloadHandler = this;
MauiHotReloadHelper.AddActiveView(ihr);
Expand All @@ -50,8 +49,8 @@ void SetView(IElement? view, bool forceRefresh = false)
currentPlatformView?.RemoveFromSuperview();
currentPlatformView = null;

if (IsViewLoaded && _view != null)
LoadPlatformView(_view);
if (IsViewLoaded && view is not null)
LoadPlatformView(view);
}

internal UIView LoadFirstView(IElement view)
Expand All @@ -63,8 +62,8 @@ internal UIView LoadFirstView(IElement view)
public override void LoadView()
{
base.LoadView();
if (_view != null && Context != null)
LoadPlatformView(_view);
if (CurrentView is IElement view && Context != null)
LoadPlatformView(view);
}

void LoadPlatformView(IElement view)
Expand All @@ -83,7 +82,7 @@ protected virtual UIView CreatePlatformView(IElement view)
_ = Context ?? throw new ArgumentNullException(nameof(Context));
_ = _view ?? throw new ArgumentNullException(nameof(view));

return _view.ToPlatform(Context);
return view.ToPlatform(Context);
}

public override void ViewDidLayoutSubviews()
Expand Down

0 comments on commit ec3d854

Please sign in to comment.