Fix: Null Reference Exception in ShellContentFragment.Destroy #29713

postalservice14 · 2025-05-28T13:08:25Z

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description of Change

This PR fixes a null reference exception in the ShellContentFragment.Destroy method for Android. The issue occurred when attempting to call RemoveAppearanceObserver on a possibly null ShellContext. The fix adds a null-conditional operator to safely access ShellContext before invoking the method.

Issues Fixed

Fixes #29712

…llContentFragment

Copilot

Pull Request Overview

This PR fixes a null reference exception occurring in the ShellContentFragment.Destroy method on Android by adding a null-conditional operator.

Updates the observer removal call to handle a potentially null _shellContext.

Comments suppressed due to low confidence (1)

src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellContentFragment.cs:176

Using the null-conditional operator bypasses the observer removal if _shellContext is null. Please confirm that skipping the removal in such cases is an intentional design decision and does not lead to lingering observers.

((IShellController)_shellContext?.Shell)?.RemoveAppearanceObserver(this);

jsuarezruiz · 2025-05-28T14:19:27Z

/azp run

azure-pipelines · 2025-05-28T14:19:50Z

Azure Pipelines successfully started running 3 pipeline(s).

rmarinho

I think using "as" seems better in this case.

Can we add a test?

rmarinho · 2025-05-28T14:50:08Z

src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellContentFragment.cs

@@ -173,7 +173,7 @@ void Destroy()
 			// to avoid the navigation `TaskCompletionSource` to be stuck forever.
 			AnimationFinished?.Invoke(this, EventArgs.Empty);

-			((IShellController)_shellContext.Shell).RemoveAppearanceObserver(this);
+			((IShellController)_shellContext?.Shell)?.RemoveAppearanceObserver(this);


Suggested change

((IShellController)_shellContext?.Shell)?.RemoveAppearanceObserver(this);

(_shellContext?.Shell as IShellController)?.RemoveAppearanceObserver(this);

@rmarinho Great suggestion on the as.

Regarding the test: I'd love to but, I can't find any existing tests around this, do you have any suggestions on how to test this scenario?

Thanks!

farneman · 2025-07-02T21:43:13Z

@rmarinho, @jsuarezruiz, @StephaneDelcroix or @tj-devel709 What can we do to help get this moving? Our app is seeing increased problems as a result of this root issue so we'd really like to get it solved.

It doesn't appear most of the checks on the PR ever reported in—how can those be restarted?

nbuchert · 2025-07-08T07:09:11Z

We have experienced this issue as well in our first .NET 9/MAUI app release 10 days ago.
According to Google this is especially related to devices with 3-4 GB RAM.
Is there any expected release date for this fix, as we are having 650+ users that are experiencing this crash?

MartyIX · 2025-07-08T07:14:20Z

What can we do to help get this moving?

I believe that this is what needs to be done: #29713 (review)

postalservice14 · 2025-07-08T12:38:37Z

What can we do to help get this moving?

I believe that this is what needs to be done: #29713 (review)

@MartyIX I asked for help on the testing here: #29713 (comment).

I wasn't sure how to test it and couldn't find any existing tests around that behavior to modify. Would appreciate some help in that area if someone has the expertise.

MartyIX · 2025-07-08T12:45:09Z

@jsuarezruiz @rmarinho Would you help with the test please?

jsuarezruiz · 2025-07-18T12:36:26Z

@jsuarezruiz @rmarinho Would you help with the test please?

Yes, let me assigned to the PR to include tests.

PureWeen · 2025-07-18T16:41:08Z

/rebase

PureWeen · 2025-07-18T17:23:15Z

/azp run

azure-pipelines · 2025-07-18T17:23:37Z

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz · 2025-07-21T09:23:32Z

@jsuarezruiz @rmarinho Would you help with the test please?

Yes, let me assigned to the PR to include tests.

@postalservice14 Could you try adding something like?

[Fact(DisplayName = "ShellContentFragment.Destroy handles null _shellContext gracefully")]
public async Task ShellContentFragmentDestroyHandlesNullShellContext()
{
	SetupBuilder();

	var shell = await CreateShellAsync(shell =>
	{
		shell.Items.Add(new TabBar()
		{
			Items =
			{
				new ShellContent()
				{
					Route = "Item1",
					Content = new ContentPage { Title = "Page 1" }
				},
				new ShellContent()
				{
					Route = "Item2", 
					Content = new ContentPage { Title = "Page 2" }
				},
			}
		});
	});

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

		// Navigate to trigger fragment creation
		await shell.GoToAsync("//Item2");
		await OnNavigatedToAsync(shell.CurrentPage);

		// Test normal destruction - should work without issues
		await shell.GoToAsync("//Item1");
		await OnNavigatedToAsync(shell.CurrentPage);

		// Test null context scenario
		var exception = Record.Exception(() => 
		{
			// Create fragment with null context - this should not throw
			Page page = new ContentPage();
			var fragment = new ShellContentFragment((IShellContext)null, page);
          
			// Dispose the fragment which calls Destroy internally
			// This validates the null-conditional operators in Destroy method
			fragment.Dispose();
		});

		// Verify no exception was thrown - validates (_shellContext?.Shell as IShellController)?.RemoveAppearanceObserver(this);
		Assert.Null(exception);
	});
}

Here:

maui/src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.Android.cs

Line 4 in 3f5adff

using Android.Views;

…ntext gracefully

postalservice14 · 2025-07-22T13:25:24Z

@dotnet-policy-service agree company="Ramsey Solutions"

…ntext gracefully

azure-pipelines · 2025-07-22T15:41:44Z

Commenter does not have sufficient privileges for PR 29713 in repo dotnet/maui

jsuarezruiz · 2025-07-23T10:05:33Z

/azp run

azure-pipelines · 2025-07-23T10:05:57Z

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz · 2025-07-24T10:26:58Z

Running the failed Android UITest again, but is not related with the changes.

PureWeen · 2025-07-24T12:08:59Z

src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellContentFragment.cs

@@ -173,7 +173,7 @@ void Destroy()
 			// to avoid the navigation `TaskCompletionSource` to be stuck forever.
 			AnimationFinished?.Invoke(this, EventArgs.Empty);

-			((IShellController)_shellContext.Shell).RemoveAppearanceObserver(this);
+			(_shellContext?.Shell as IShellController)?.RemoveAppearanceObserver(this);


Can we also add a check on this destroy method so that it only runs once?

It's fine to leave this code but I just worry that this destroy method is being called twice which might just manifest as a different problem down the road

Let me check it.

Fix null reference exception when removing appearance observer in She…

79776c2

…llContentFragment

Copilot AI review requested due to automatic review settings May 28, 2025 13:08

postalservice14 requested a review from a team as a code owner May 28, 2025 13:08

postalservice14 requested review from StephaneDelcroix and tj-devel709 May 28, 2025 13:08

Copilot AI reviewed May 28, 2025

View reviewed changes

dotnet-policy-service bot added the community ✨ Community Contribution label May 28, 2025

jsuarezruiz added area-controls-shell Shell Navigation, Routes, Tabs, Flyout platform/android labels May 28, 2025

rmarinho reviewed May 28, 2025

View reviewed changes

Change to use as syntax

868e64a

jsuarezruiz self-assigned this Jul 18, 2025

PureWeen added this to the .NET 9 SR10 milestone Jul 18, 2025

PureWeen added this to MAUI SDK Ongoing Jul 18, 2025

github-project-automation bot moved this to Todo in MAUI SDK Ongoing Jul 18, 2025

PureWeen moved this from Todo to Ready To Review in MAUI SDK Ongoing Jul 18, 2025

Add test to verify ShellContentFragment.Destroy handles null _shellCo…

bfc03af

…ntext gracefully

Merge branch 'dotnet:main' into RPLUS-76242/fix-npe

d180074

mattleibow previously approved these changes Jul 22, 2025

View reviewed changes

github-project-automation bot moved this from Ready To Review to Approved in MAUI SDK Ongoing Jul 22, 2025

dotnet deleted a comment from azure-pipelines bot Jul 22, 2025

dotnet deleted a comment from mattleibow Jul 22, 2025

Add test to ensure ShellContentFragment.Destroy handles null _shellCo…

1bf62b0

…ntext gracefully

postalservice14 dismissed mattleibow’s stale review via 1bf62b0 July 22, 2025 15:30

jsuarezruiz approved these changes Jul 23, 2025

View reviewed changes

PureWeen requested changes Jul 24, 2025

View reviewed changes

github-project-automation bot moved this from Approved to Changes Requested in MAUI SDK Ongoing Jul 24, 2025

	((IShellController)_shellContext?.Shell)?.RemoveAppearanceObserver(this);
	(_shellContext?.Shell as IShellController)?.RemoveAppearanceObserver(this);

Fix: Null Reference Exception in ShellContentFragment.Destroy #29713

Are you sure you want to change the base?

Fix: Null Reference Exception in ShellContentFragment.Destroy #29713

Uh oh!

Conversation

postalservice14 commented May 28, 2025

Description of Change

Issues Fixed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

jsuarezruiz commented May 28, 2025

Uh oh!

azure-pipelines bot commented May 28, 2025

Uh oh!

rmarinho left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rmarinho May 28, 2025

Choose a reason for hiding this comment

Uh oh!

postalservice14 May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

farneman commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nbuchert commented Jul 8, 2025

Uh oh!

MartyIX commented Jul 8, 2025

Uh oh!

postalservice14 commented Jul 8, 2025

Uh oh!

MartyIX commented Jul 8, 2025

Uh oh!

jsuarezruiz commented Jul 18, 2025

Uh oh!

PureWeen commented Jul 18, 2025

Uh oh!

PureWeen commented Jul 18, 2025

Uh oh!

azure-pipelines bot commented Jul 18, 2025

Uh oh!

jsuarezruiz commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

postalservice14 commented Jul 22, 2025

Uh oh!

azure-pipelines bot commented Jul 22, 2025

Uh oh!

jsuarezruiz commented Jul 23, 2025

Uh oh!

azure-pipelines bot commented Jul 23, 2025

Uh oh!

jsuarezruiz commented Jul 24, 2025

Uh oh!

PureWeen Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

jsuarezruiz Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rmarinho left a comment •

edited

Loading

postalservice14 May 28, 2025 •

edited

Loading

farneman commented Jul 2, 2025 •

edited

Loading

jsuarezruiz commented Jul 21, 2025 •

edited

Loading