[Android] [Shell] replace navigation leaks current page

[albyrock87]

Description

Shell is leaking the page on android upon replace on Android.

• from a shell content
• push a page GoToAsync("child")
• from the child, replace it GoToAsync("../other-child")
• now check if child page is still alive

Steps to Reproduce

This is the UITest to prove the leak.

namespace Maui.Controls.Sample.Issues
{
	[Issue(IssueTracker.Github, 99999, "Shell leaks when replacing a page", PlatformAffected.Android)]
	public class Issue99999 : Shell
	{
		public Issue99999()
		{
			var mainContent = new ShellContent
			{
				ContentTemplate = new DataTemplate(() => new InitialPage()),
				Title = "Initial",
				Route = "initial"
			};
			
			Items.Add(mainContent);
			Routing.RegisterRoute("Issue99999_child", typeof(ChildPage));
			Routing.RegisterRoute("Issue99999_replace", typeof(ReplacePage));
		}
	}
	
	file class InitialPage : ContentPage
	{
		public WeakReference<Page> ChildPageReference { get; set; }

		public InitialPage()
		{
			Padding = 24;
			Content = new Button
			{
				Text = "Go to child page",
				AutomationId = "goToChildPage",
				VerticalOptions = LayoutOptions.Start,
				Command = new Command(() => Shell.Current.GoToAsync("Issue99999_child"))
			};
		}
	}
	
	file class ChildPage : ContentPage
	{
		public ChildPage()
		{
			Padding = 24;
			Content = new Button
			{
				Text = "Replace",
				AutomationId = "replace",
				VerticalOptions = LayoutOptions.Start,
				Command = new Command(() => Shell.Current.GoToAsync("../Issue99999_replace"))
			};
		}

		protected override void OnParentSet()
		{
			base.OnParentSet();
			
			if (Parent is ShellSection section)
			{
				var initialPage = (section.CurrentItem as IShellContentController).Page as InitialPage;
				initialPage!.ChildPageReference = new WeakReference<Page>(this);
			}
		}
	}
	
	file class ReplacePage : ContentPage
	{
		public ReplacePage()
		{
			Padding = 24;

			Button checkRefButton = null;
			checkRefButton = new Button
			{
				Text = "Check reference",
				AutomationId = "checkReference",
				VerticalOptions = LayoutOptions.Start,
				Command = new Command(async () =>
				{
					GC.Collect();
					GC.WaitForPendingFinalizers();
					await Task.Yield();

					var section = (ShellSection)Parent;
					var initialPage = (InitialPage)(section.CurrentItem as IShellContentController).Page;
					checkRefButton.Text = initialPage.ChildPageReference.TryGetTarget(out var page) ? "alive" : "gone";
				})
			};

			var backButton = new Button
			{
				Text = "Go back",
				AutomationId = "goBack",
				VerticalOptions = LayoutOptions.Start,
				Command = new Command(() => Shell.Current.GoToAsync(".."))
			};

			Content = new VerticalStackLayout
			{
				checkRefButton,
				backButton
			};
		}
	}
}

Link to public reproduction project repository

No response

Version with bug

8.0.91 SR9.1

Is this a regression from previous behavior?

Not sure, did not test other versions

Last version that worked well

Unknown/Other

Affected platforms

Android

Affected platform versions

No response

Did you find any workaround?

Navigate back. Then push the new page.
But obviously the user experience is impacted by that.

Relevant log output

No response

[albyrock87]

@dawin-steng reported the issue on Nalu and added an interesting detail:

Looking at gcdumps, the problem seems to be that the shell is holding a reference to the page
List<ValueTuple<Microsoft.Maui.Controls.IAppearanceObserver, Microsoft.Maui.Controls.Element>>

[ninachen03]

Could you provide us with a sample project so we can investigate it further? Looking forward to your reply!

[bhavanesh2001]

@albyrock87 This is nothing new and not specific to Shell, I think. One way around is to tear down the child page yourself explicitly before navigating to another page.

[dawin-steng]

@bhavanesh2001 The problem is not with the page content, but the page itself. Shell keeps a reference the page, even though the page was removed from the NavigationStack. Thus, even if you tear down the page content as you suggest, the page itself will never be garbage collected.

The issue is specific to shell and to the scenario of removing a page from the NavigationStack without PopAsync.
This can be either a navigation forward with ".." in the route (as described above), or by manually pushing a new page and then removing the previous one.

For the following examples, consider this a navigation from a Page2, with the goal to remove itself from the NavigationStack when navigating to Page3:

await Shell.Current.Navigation.PushAsync(new Page3());
Shell.Current.Navigation.RemovePage(this); // this causes the reference to Page2 to remain

Other scenarios clean up references, but do impact the user experience. The mentioned workaround of first popping then pushing

await Shell.Current.Navigation.PopAsync();
await Shell.Current.Navigation.PushAsync(new Page3());

... or using the inverted order, (inserting the page beneath the current one, then popping the current one)

Shell.Current.Navigation.InsertPageBefore(new Page3(), this);
await Shell.Current.Navigation.PopAsync();

Note that this issue is specific to shell, because the erroneous scenario is fine using NavigationPages

await App.Current.MainPage.Navigation.PushAsync(new Page3());
App.Current.MainPage.Navigation.RemovePage(this);

One more note: I assume that #21403 was referring to the same problem.

[albyrock87]

The repro is included in issue description, ready to be used as UITest.