Repository navigation
Conversation
|
@Nezz Confirmed against current native source on an API 35 Pixel 6 emulator: the unpatched one-task case fails in all three trials (two fatal isViewClipped exceptions and one mounting-error/stalled-update run). The patched build passes three 20-second trials (39/40/40 updates) and a separate 60-second run (122 updates), with no crashes or mounting errors. The two-task control also continued past 500 updates. Could you retest this targeted fallback on your API 36 setup? |
|
Adding a production data point in case it helps prioritize this. We see a related signature on Android:
It looks like the same merged-transaction ordering problem as #57017 (delete + recreate of a tag ending up out of order). If this PR is expected to cover the |
|
@GersonRocha9 This is expected to cover Our recorded unpatched, current-source reproduction also produced This patch preserves native view lifetimes; it does not clamp insertion indices or suppress exceptions. Could you share the full This PR targets main; a 0.86.3 backport has not been validated. I would keep your production case unconfirmed until that evidence or retest is available. |
|
@GersonRocha9 Follow-up: I have now validated the proposed index-1 failure chain locally, without needing an app reproducer from you. The added coverage is in 83a4269. Using React Native's real Java mounting classes, a merged delete/recreate with Delete moved to the end loses the child's ViewState. Moving that child into an empty parent then skips the insert at index 0; inserting its sibling at index 1 throws the wrapped Preserving the mutation order, as this PR does, makes the same sequence succeed: both children remain registered and correctly ordered, the source parent is empty, and no soft exceptions are emitted. I added four Java-side characterization tests plus a C++ test confirming that this merged lifetime requests strict order. No additional production change or bounds-clamping workaround was needed for this mechanism. The full Android JVM suite passes (861 tests), as do all 11 C++ order-selection tests. The Java tests use explicit instruction buffers under Robolectric, not a full C++/JNI pipeline or your 0.86.3 app. They establish this causal mechanism rather than proving that every similar production stack has the same cause. |
Summary
Fixes #59724.
MountingTransaction::mergeWithcan concatenate a delete and a later create for the same native tag. Android normally moves deletes to the end of the encoded batch, so the create can be skipped while the old view is registered and the trailing delete then unregisters the newly reinserted view. Subsequent FlatList updates fail with missing view state, an existing parent, and finallyReactViewGroup.isViewClipped.Detect delete-then-create lifetimes before collecting mount items and use the existing order-preserving encoder for those batches. Preserve the old grouping for safe batches, and skip hash-set allocation for batches without a delete followed by any create. Virtual views are included because their registry state also has a lifetime. The experimental global flag and Java crash checks are unchanged.
Changelog
[ANDROID] [FIXED] - Preserve native view lifetimes when merged Fabric transactions delete and recreate the same tag
Test Plan
debaf42before enabling the fix: three one-task trials all failed, two with the reported fatalisViewClippedstack and one with mounting errors/stalled updates.ShadowViewMutationandMountingTransactionimplementations: safe batches, reparenting, virtual views, same-tag recreation, repeated/interleaved lifetimes, merged transactions, and recreation followed by reparenting into an empty container. These test the encoder's order-selection helper, not a mock copy of it.IntBufferBatchMountItem,SurfaceMountingManagerandReactViewManager. Moving Delete last loses the recreated child's ViewState, skips its subsequent insert at index 0, then throwsaddViewAt ... index 1with anIndexOutOfBoundsExceptioncause. Preserved order succeeds with both clipping enabled and disabled, keeps both children in order, and emits no soft exceptions.git diff --checkpass.The release reproduction ran on a dedicated arm64 API 35 Pixel 6 emulator. It is the report's JS fixture with only current-source integration adjustments: the current activity delegate API, matching NDK 28.2 and source-built Hermes compiler. The original report used API 36. No physical-device, all-ABI, or full renderer GTest-suite coverage is claimed. The eleven host GTests used a focused external CMake harness because the existing Android JNI instrumentation test target is not available in the OSS Gradle build.
The new Java-side tests run under Robolectric with manually specified mount-instruction buffers; they do not execute the C++ encoder through JNI. Together with the C++ order-selection test, they validate the proposed skipped-insert mechanism, not the root cause of every production
addViewAtcrash or an RN 0.86.3 backport.