Conversation
… is attached PortalViewManager.createFragment committed the PortalFragment transaction asynchronously. FragmentManager resolves the container view by id when the transaction executes on the next main-loop iteration and throws IllegalArgumentException: No view found for id 0x… for fragment PortalFragment when the container is not part of the activity's window at that moment: - React dropped the PortalView before the transaction ran (mount and unmount in the same batch). onDropViewInstance could not cancel the pending add, because Fragment.parentFragmentManager throws until the add has executed and the catch swallowed it. - The view exists but is not attached yet, e.g. a react-native-screens screen that is still animating in when the PortalView mounts (seen in production when a portal screen is re-opened with a cached portal config), or it was detached while React still has the subtree mounted. Commit the transaction with commitNowAllowingStateLoss from a Runnable posted on the container view instead. View.post defers the runnable until the view is attached and always dispatches it through the main handler, so the commit never nests inside another FragmentManager transaction. When it runs, it re-checks that the view was not dropped and is attached. viewState.fragment is only set after a successful add, so onDropViewInstance can use parentFragmentManager without the try/catch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@dmvk is attempting to deploy a commit to the Ionic Team on Vercel. A member of the Team first needs to authorize it. |
Extract the "commit once the container is attached and still ours" logic into runWhenAttached so it can be exercised without PortalFragment (which would boot a Capacitor bridge and WebView), and cover it with Robolectric against the real androidx FragmentManager: - control test reproducing the production failure: an async commit whose container was removed before it ran throws "No view found for id" - commits once the looper runs when the container is attached - skips the commit when the view was dropped before it ran - waits for the container to attach before committing - does not throw when the container is detached while still current, and commits once it comes back The library module had no unit test setup; this adds junit + robolectric as test dependencies and a dedicated test-android CI job that runs testDebugUnitTest on every push. It is a separate job on purpose: the build-android job is replayed from the turbo cache whenever yarn.lock is unchanged, so a step gated on that cache would be skipped exactly when android/ changed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
dmvk
force-pushed
the
fix-android-portal-fragment-container-race
branch
from
September 30, 2026 13:36
b640785 to
735a488
Compare
This branch has not been deployed
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.
Problem
On Android,
PortalViewcan crash the host app with an uncaughtSeen in production (Android 16,
@ionic/portals-react-native0.9.0, React Native 0.81.5 with the New Architecture, react-native-screens 4.16.0). Both occurrences we have follow the same shape: the user leaves a screen that hosts aPortalView, comes back to it within a second or two, and the app dies about 150 ms after the tap. On the re-open the portal config is already cached, so thePortalViewmounts a few milliseconds after the screen is pushed.mainhas the samePortalView.ktas 0.9.0 and 0.9.1, so this applies to the current release.Root cause
PortalViewManager.createFragmentcommits thePortalFragmenttransaction asynchronously:activity.supportFragmentManager .beginTransaction() .replace(viewId, portalFragment, "$viewId") .commit()FragmentManagerruns that on the next main-loop iteration and only then resolves the container by id (FragmentContainer.onFindViewById→Activity.findViewById). If the container is not part of the activity's window at that moment,FragmentStateManager.createViewthrows the exception above. There are three ways to get there from a React Native app:createcommand runs as soon as the JS side mounts the view, but the surrounding screen may still be animating in. react-native-screens attaches a screen's fragment view via its own transaction, and if that lands after ours executes,findViewByIdfails. This is the case we observe.PortalViewin one batch.onDropViewInstancetries to cancel throughfragment.parentFragmentManager, but that property throws until the add has executed, so thetry/catchswallowed it and the orphaned add ran anyway. (Queuing a remove behind the add would not help either:removeRedundantOperationsAndExecuteexecutes non-reorderable, non-pop records one at a time.)PortalViewmounting in that window hits the same exception.Fix
Commit the transaction with
commitNowAllowingStateLoss()from aRunnableposted on the container view, and re-check state when it runs:View.post()queues the runnable until the view is attached (it goes through the view'sHandlerActionQueue, which re-posts on the main handler at attach time), and always dispatches through the main handler. So the commit never nests inside anotherFragmentManagertransaction that may be executing, which is the hazard a plaincommitNowat command time would have with react-native-screens' owncommitNowtransactions.viewState.fragmentis only set after a successful add, soonDropViewInstancecan useparentFragmentManagerwithout thetry/catch.Behaviour change to be aware of: the fragment is now added synchronously inside the posted runnable rather than on
FragmentManager's own schedule, andcommitAllowingStateLossreplacescommiton the remove path (the previous code caught and logged the state-loss exception, so nothing that used to succeed changes).Alternative considered
setReorderingAllowed(true)on both transactions makesFragmentManagerbatch a pending add with a later remove and skip view creation entirely, which fixes case 2 on its own. It does nothing for case 1 (the container exists but is not in the window), which is the one we see in production, so it was not enough.Testing
Regression tests (new)
The library module had no unit test setup, so this PR adds JUnit + Robolectric to
android/build.gradleand a dedicatedtest-androidCI job that runs./gradlew :ionic_portals-react-native:testDebugUnitTeston every push and prints the JUnit summary. It is a separate job rather than a step inbuild-androidon purpose: that job is replayed from the turbo cache wheneveryarn.lockis unchanged (theandroidentry inturbo.jsoninputs does not hash the directory's contents), so a step gated on the cache would be skipped exactly whenandroid/changed. You may want to fix that input glob separately; I left it alone here. Robolectric is used because the behaviour under test isFragmentManager+ view attachment semantics, which need a real activity/window but not an emulator.To make the logic testable without booting a Capacitor bridge and WebView, the "commit once attached and still ours" part is extracted into
runWhenAttached(view, isCurrent, action);createFragmentcalls it with the same checks as before.RunWhenAttachedTestdrives the real androidxFragmentManagerwith plainFragments:asyncCommitThrowsWhenTheContainerWasRemovedBeforeItRancommit(), container removed, looper runs) throwsNo view found for idunder Robolectric, so the harness reproduces the production crash.commitsOnceTheLooperRunsWhenTheContainerIsAttachedskipsTheCommitWhenTheViewWasDroppedBeforeItRanwaitsForTheContainerToAttachBeforeCommittingView.postqueue).doesNotThrowWhenTheContainerIsDetachedWhileStillCurrentCI run with the tests (
test-android: 5 tests, 0 failures): https://github.com/dmvk/ionic-portals-react-native/actions/runs/36722825635Other verification
build-androidCI job (example app, Kotlin 2.1.20, compileSdk 36), first run before the tests were added: https://github.com/dmvk/ionic-portals-react-native/actions/runs/36718978352removeRedundantOperationsAndExecute/executeOpsTogether(batching semantics) andandroid.view.View#post/HandlerActionQueue#executeActions(pre-attach posts are re-posted through the handler, not run synchronously).yarn example android, that would be a welcome check.Reproducing the crash without the fix
Case 1 is the one with production evidence: host the
PortalViewin a@react-navigation/native-stackscreen that renders it immediately on mount, then pop and re-push that screen quickly (the faster thePortalViewmounts after the push, the more likely the crash). Case 2 needs two React commits back to back so the mount and the unmount land in one native batch, for example aPortalViewwhose parent unmounts it from a mount effect; separate taps on a toggle button are usually too far apart to hit it.🤖 Generated with Claude Code