fix(android): stop the FragmentManager driving fragments with a dead entry - #11352
Open
edusperoni wants to merge 1 commit into
Open
fix(android): stop the FragmentManager driving fragments with a dead entry#11352edusperoni wants to merge 1 commit into
edusperoni wants to merge 1 commit into
Conversation
…entry A fragment can outlive the BackstackEntry it is bound to. Navigation only evicts fragments through transaction.replace(containerViewId, ...), so any fragment whose container no longer matches the frame's current container - left over from an activity recreation, a frame reset, or an interrupted navigation - stays added to the FragmentManager. _removeEntry then clears resolvedPage and drops entry.fragment without telling the FragmentManager, and the next transaction happily drives that fragment back through onCreateView. Every callback on that path either reported the condition with Trace.error - which routes to the error handler, rethrows, and becomes a fatal exception across the JNI boundary - or dereferenced the missing page directly, so the stale fragment took the app down instead of being discarded. - _removeEntry: detach the fragment's callbacks and remove it from the FragmentManager when it is still added, so it can neither be driven again nor resurrect the torn down page - findPageForFragment: discard an unclaimed restored fragment instead of throwing, and widen the entry lookup through _findEntryForTag so fragments restored from the backstack or the navigation queue are matched instead of treated as orphans - onDestroy: report a missing entry without throwing - onResume: bail out when the entry or page is gone rather than reading entry.resolvedPage.frame - onPause/onDestroyView: tolerate a missing frame
|
View your CI Pipeline Execution ↗ for commit f67ee37
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Pull request overview
Prevents Android’s FragmentManager from driving orphaned fragments after their navigation entries are discarded.
Changes:
- Removes still-added fragments when entries are discarded.
- Expands restored-fragment lookup across backstack and navigation queues.
- Adds null-safe lifecycle handling for stale fragments.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/core/ui/frame/index.android.ts |
Detaches and removes fragments for discarded entries. |
packages/core/ui/frame/frame-helper-for-android.ts |
Adds orphan cleanup and lifecycle guards. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } else { | ||
| // Android also restores fragments that were only in the backstack or still queued, so | ||
| // widen the lookup before treating this fragment as an orphan. | ||
| entry = frame._findEntryForTag(fragmentTag); |
Comment on lines
+62
to
+63
| Trace.write(`Could not find a page for ${fragmentTag}. Discarding orphaned fragment.`, Trace.categories.NativeLifecycle, Trace.messageType.error); | ||
| removeFragmentIfAdded(fragment); |
Comment on lines
+92
to
+93
| callbacks.entry = null; | ||
| callbacks.frame = null; |
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.
PR Checklist
What is the current behavior?
A
FragmentClasscan outlive theBackstackEntryit is bound to, and the FragmentManager then drives it through the lifecycle against that dead entry. The reported symptom is a fatal:Note
<null>in the fragment'stoString: the fragment'sentryis intact, onlyresolvedPageis gone.How the fragment gets stranded. Navigation only evicts fragments via
transaction.replace(this.containerViewId, ...), which removes added fragments in that container.Frame._removeEntrythen clearsresolvedPage(FrameBase._removeEntry) and dropsentry.fragment, but never tells the FragmentManager anything — it assumes thereplacealready did. That assumption breaks for any fragment whose container is no longer the frame's current container: leftovers from an activity recreation, a frame/root-view reset, or a navigation that was interrupted mid-flight. Such a fragment staysmAdded, somoveToExpectedState()keeps promoting it, and it getsonCreateViewagain after its entry has been discarded.Why it is fatal rather than a no-op. Every callback on that path either reported the condition through
Trace.error— which routes toDefaultErrorHandler, rethrows, and surfaces as an uncaught exception across the JNI boundary — or dereferenced the missing page directly:onCreateView— fixed in fix(android): don't crash in Fragment.onCreateView on stale-fragment race #11264, but that only moved the crash one step later, since the fragment continues to…onResume—this.entry.resolvedPage.framethrows aTypeErroron the very same stale fragmentonDestroy—Trace.erroron a missing entry, which is exactly what a discarded fragment hits on teardownonPause/onDestroyView—this.frame.nativeViewProtectedwith no framefindPageForFragment—throw new Error('Could not find a page for <tag>')for a restored fragment no live entry claimsThis correlates strongly with memory pressure: the tighter memory gets, the more the OS destroys and recreates activities, which is what strands the fragments in the first place. It was reported from production with 300+ occurrences on low-RAM (4 GB) devices, mostly on resume after the app had been idle.
What is the new behavior?
The stale fragment is discarded instead of taking the app down, and it stops being stale in the first place:
Frame._removeEntry— when the discarded entry still has a fragment, detach the fragment's callbacks (entry/frame) and remove it from its FragmentManager if it is still added. This is the actual leak fix: the guard only fires for fragments that are still added while their entry is being discarded, i.e. exactly the orphans. In normal forward/back/clearHistory/backstackVisible: falsenavigation the outgoing fragment has already been evicted by thereplacebeforesetCurrentruns, soisAdded()is false and nothing changes.findPageForFragment— widen the entry lookup through the (previously unused)_findEntryForTag, which also searches the backstack and the navigation queue, so fragments Android restored from those are matched rather than declared orphans. When there genuinely is no entry, log and remove the fragment instead of throwing.onDestroy/onResume/onPause/onDestroyView— treat a missing entry, page or frame as the recoverable state it is.onResumein particular had to be guarded, or theonCreateViewfix from fix(android): don't crash in Fragment.onCreateView on stale-fragment race #11264 just relocates the crash.Removals are always committed with
commitAllowingStateLoss(): callers can be inside a FragmentManager transaction (findPageForFragmentruns fromonCreate,_removeEntryfrom a transition listener), wherecommitNowthrows "FragmentManager is already executing transactions".Also considered and left out: extending the restored-fragment cleanup in
ActivityCallbacksImplementation.onCreate(which deliberately skipsfragment*tags) to drop unclaimed NativeScript fragments. Having each orphan remove itself in its ownonCreateis ordering-independent and also covers child FragmentManagers, which that cleanup does not walk.No unit tests:
packages/corespecs cover parser/pure-logic modules only and nothing platform-specific, and these are FragmentManager lifecycle paths. Verification is on device — activity destroy/recreate (Don't keep activities) with a navigation in flight, plus tabs/bottom-navigation nested frames on resume.