Don't resurrect layout animations cancelled before their start lambda ran#9660
Open
bartlomiejbloniarz wants to merge 3 commits into
Open
Don't resurrect layout animations cancelled before their start lambda ran#9660bartlomiejbloniarz wants to merge 3 commits into
bartlomiejbloniarz wants to merge 3 commits into
Conversation
… ran
On Android, pullTransaction can run on the JS thread, so animation starts
are scheduled onto the UI thread. If the view is removed before the
scheduled start runs, maybeCancelAnimation finds no layoutAnimations_
entry to erase and the cancellation is lost - the stale start then
re-creates the animation for a view whose Remove+Delete are already in
the mount-item queue, and its updates race the queued Delete
("Unable to find viewState for tag X", #7493).
Pending starts now capture a per-tag handle that cancellations
invalidate; a stale start bails out instead of resurrecting the
animation. Includes a repro example (interrupted exiting animation).
Closes #7493
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Closes #7493
Summary
Fixes another cause of #7493 (
RetryableMountingLayerException: Unable to find viewState for tag X. Surface stopped: false) that reproduces onmain(despite the fix from #8083).If new reports come in after this lands, we can reopen.
The bug is Android-specific. On Android, Fabric uses a push model:
pullTransactioncan run onthe JS thread, which is why starting a layout animation (entering, exiting or layout) can't
happen inline —
pullTransactionschedules a start lambda onto the UI thread, and only thatlambda creates the
layoutAnimations_entry. (On iOS pulls happen on the main thread, so thestart and any later cancellation are naturally ordered and none of this can race.)
That asynchrony leaves a gap: if the view is removed before the start lambda runs — easy when
the UI thread is stalled, since the removal is processed by another
pullTransactionon the JSthread —
maybeCancelAnimationfinds no entry to erase and silently does nothing. The lambdathen runs anyway and starts the animation for a view whose
Remove+Deleteare already sittingin Android's mount-item queue. The first progress update is emitted while the view still looks
mounted (
preserveMountedTagschecks the host view registry, and theDeletehasn't beenexecuted yet — so it passes, correctly), and the next synchronous mount drains the queue in
FIFO order:
Deletefirst, then the freshUpdate.getViewStatethrows.Depending on timing this surfaces in two ways (both reproduced, both gone with this fix): the
mounting-layer exception above, or — if no mount happens while the view still resolves as
mounted — a zombie animation that finishes ~a second later and trips
react_native_assert(parent && "Parent node is nullptr")inhandleRemovals, because its nodewas already flushed out of the tree once.
The interleaving
Captured from the repro on a Pixel 5 emulator (Android's JS and UI threads; exiting variant):
Remove(item)pulled: item hasexiting→ Remove/Delete withheld,startExitingAnimationschedules lambda L on the UI thread. NolayoutAnimations_entry exists yet.shouldAnimate=false(react-native-screens pop /skipExiting):endAnimationsRecursivelyforce-ends the item.maybeCancelAnimation→ no entry → no-op.Remove+Delete(item)go into the queued mount batch.createLayoutAnimationcreates the entry inlayoutAnimations_; the exiting config was never cleared on the force-end path, so the animation starts and writes its first frame into the update map.Update(item)—preserveMountedTagspasses, theDeleteis still queued.performOperations(Android mounts synchronously from event handling) → FIFO drain executes the queuedDelete(item), then theUpdate(item)→ 💥Unable to find viewState for tag XThe entering variant has the same shape but is defused today by an accident: the plain-removal
path calls
clearLayoutAnimationConfig, so the resurrected start hits the missing-configearly-return. The force-end paths don't clear configs, so the exiting variant has no such luck.
Rather than rely on that side effect, this fixes the actual problem — a cancellation that can
race the start and lose.
The fix
Android-only (
#ifdef ANDROID, like the rest of the thread-related special-casing in thesefiles). Both proxies (
_Legacyand_Experimental) get the same treatment, with the sharedbits in
LayoutAnimationsProxyCommon.h:pendingStarts_count is bumped and thecurrent handle is captured into the lambda;
maybeCancelAnimationbumps the handle, invalidating starts still in flight for that tag;(
consumeIsCancelled; the exiting lambda also clears the animation config on that path,mirroring the non-animated removal path).
Reproduction & testing
New example: [LA] Interrupted exiting animation (#7493) in
apps/common-app. It needs nolibrary modifications: each cycle blocks the UI thread for ~600ms with a busy-wait worklet
(standing in for a real main-thread stall), unmounts the exiting item while blocked, then force-ends it via a
skipExitingwrapper unmount in a second commit (two separate commits matter — React'sautomatic batching would otherwise fold both unmounts into one transaction, which takes a
harmless path).
On an API 35 emulator: without the fix the example dies within a few seconds of being
opened — the mounting-layer exception when scrolling, the
handleRemovalsassert when leftalone. With the fix it runs indefinitely.
Notes
pull model (Props 2.0); together with [LA] Add check if view is mounted on Android #8083 this removes the known ways reanimated itself
emits an
Updatefor a view whoseDeleteis already committed.