[net11.0][Android] Shell: Fix fragment teardown crash (port of #37048) - #37121
Closed
davidnguyen-tech wants to merge 1 commit into
Closed
[net11.0][Android] Shell: Fix fragment teardown crash (port of #37048)#37121davidnguyen-tech wants to merge 1 commit into
davidnguyen-tech wants to merge 1 commit into
Conversation
Port of dotnet#37048 from release/11.0.1xx-preview7 to net11.0. ShellItemHandlerAdapter.Dispose() and ShellSectionHandlerAdapter.Dispose() disposed the wrapper fragment's managed peer while AndroidX FragmentManager still owned the Java fragment. FragmentManager later invoked OnDestroyView on a fragment whose managed peer was already gone, crashing the native-to-managed callback. Removes the premature _wrapperFragment?.Dispose() call in both adapters. Destroyed?.Invoke, DisconnectHandler() and clearing _wrapperFragment are preserved, so handler cleanup and event semantics are unchanged. (cherry picked from commit 77f3371) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 751493a4-4b7c-4eaf-88a9-fcc68ba397af
davidnguyen-tech
temporarily deployed
to
copilot-pat-pool
August 5, 2026 14:42 — with
GitHub Actions
Inactive
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 37121Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 37121" |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
davidnguyen-tech
temporarily deployed
to
copilot-pat-pool
August 5, 2026 14:43 — with
GitHub Actions
Inactive
davidnguyen-tech
temporarily deployed
to
copilot-pat-pool
August 5, 2026 14:43 — with
GitHub Actions
Inactive
davidnguyen-tech
temporarily deployed
to
copilot-pat-pool
August 5, 2026 14:46 — with
GitHub Actions
Inactive
davidnguyen-tech
temporarily deployed
to
copilot-pat-pool
August 5, 2026 14:47 — with
GitHub Actions
Inactive
davidnguyen-tech
temporarily deployed
to
copilot-pat-pool
August 5, 2026 14:47 — with
GitHub Actions
Inactive
davidnguyen-tech
marked this pull request as ready for review
August 5, 2026 14:47
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
Ports the Android Shell fragment-teardown fix to net11.0 by correcting fragment ownership during adapter disposal so AndroidX FragmentManager can complete lifecycle callbacks without the managed fragment peer being prematurely disposed (preventing deterministic device-test process crashes during teardown).
Changes:
- Remove
_wrapperFragment?.Dispose()fromShellItemHandlerAdapter.Dispose()and retain only handler disconnect +_wrapperFragment = null. - Remove
_wrapperFragment?.Dispose()fromShellSectionHandlerAdapter.Dispose()and retain only handler disconnect +_wrapperFragment = null. - Add inline comments documenting why the wrapper fragment must not be disposed by the adapter (FragmentManager-owned lifecycle).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Android.cs | Stops disposing the FragmentManager-owned ShellItemWrapperFragment during adapter teardown to avoid post-dispose native→managed lifecycle callbacks. |
| src/Controls/src/Core/Handlers/Shell/ShellSectionHandler.Android.cs | Applies the same fragment-ownership correction for ShellSectionWrapperFragment during adapter teardown. |
Contributor
|
Closing because all the preview7 fixes will be ported here #37119 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Ports #37048 ("[Android] Shell: Fix fragment teardown crash") from
release/11.0.1xx-preview7tonet11.0.#37048 fixed the crash on the preview7 branch only.
net11.0never received it, so the Android Controls device tests have been crashing there continuously since 2026-07-28 and are still crashing today.Root cause (unchanged from #37048):
ShellItemHandlerAdapter.Dispose()andShellSectionHandlerAdapter.Dispose()disposed the wrapper fragment's managed peer while AndroidXFragmentManagerstill owned the Java fragment.FragmentManagerlater invokedOnDestroyViewon a fragment whose managed peer was already gone, so the native-to-managed callback failed.The failure is deterministic — Helix retried the work item 3× on a fresh emulator each time and dead-lettered it — and it takes down the whole test process, so no results file is written at all:
Because there is no TRX, AzDO reports
failedTests: 0and the crash is invisible in test-result counts; it is only visible via the Helix work-item exit code. In build 1539159, 653 passing tests were observed in logcat and then lost.Both runtimes are affected, with different symptoms:
Mono
CoreCLR — the same failed peer resolution instead recurses until the stack is exhausted (
n_OnDestroyView×5745 /OnDestroyView()×5744, every frame the baseAndroidX.Fragment.App.Fragmenttype):Changes
Straight cherry-pick of
77f337115f0448568353e856387c937ebb50364b, applied cleanly with no conflicts. The resulting file contents are byte-identical torelease/11.0.1xx-preview7in both changed regions.ShellItemHandlerAdapter.Dispose()no longer disposes its FragmentManager-owned wrapper fragment.ShellSectionHandlerAdapter.Dispose()applies the same ownership correction.Destroyed?.Invoke,DisconnectHandler()and clearing_wrapperFragmentare all preserved, so handler cleanup and event semantics are unchanged.Validation
Evidence that this change is what separates passing from crashing runs. Helix work-item exit code for
com.microsoft.maui.controls.devicetests-Signed(-1/80= process crash, no results;1= ordinary test failures with results written):1— last good-1first bad-1-1-1most recent00In every failing run the last test to pass is
SearchHandlerRendersCorrectly, with the crash following ~1s later during Shell fragment teardown.mainis unaffected — the wrapper fragment classes do not exist there.This PR's own
maui-pr-devicetestsrun is the real check: the Android CoreCLR and Mono Controls legs should go green and, importantly, should report test results rather than dying with no TRX.Follow-up (not addressed here, deliberately)
All three new Shell wrapper fragments still lack the
(IntPtr, JniHandleOwnership)JNI activation constructor — onnet11.0and onpreview7:ShellItemWrapperFragment,ShellSectionWrapperFragment,ShellContentNavigationFragmentThe pre-existing analogues have it (
NavigationViewFragment.cs:29,MauiNavHostFragment.cs:18). This PR removes the trigger, but any other way of losing the peer (GC, process-death restore, another disposal path) would land back in the same catastrophic CoreCLR recursion instead of a readable error. Kept out of this PR to keep it a clean, low-risk port.Related: #34758 (introduced the premature dispose), #36133 (added
ShellItemWrapperFragment.OnDestroyView()), #36108, #36264.