fix(android): dispatch Rive events on the UI thread to avoid Reanimated deadlock/ANR - #445
Open
Togetic wants to merge 1 commit into
Open
fix(android): dispatch Rive events on the UI thread to avoid Reanimated deadlock/ANR#445Togetic wants to merge 1 commit into
Togetic wants to merge 1 commit into
Conversation
Rive's render thread emitted JS events inline while holding the state-machine advance lock. With Reanimated in the tree, delivery is routed through the main thread, so if the main thread was waiting on that same lock the two threads deadlocked and Android raised an ANR. Hand each event to the UI thread via UiThreadUtil.runOnUiThread so the emitting thread never holds the Rive lock across the bridge call.
3 tasks
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.
Fixes #444.
Problem
RiveReactNativeViewemits all of its JS events from whatever thread the Rive runtime calls it on:Rive's render thread runs these callbacks while holding the state-machine advance lock. With Reanimated in the tree the event's delivery is routed through the main thread, so when the main thread is itself waiting on that lock the two threads deadlock — a lock-ordering inversion that surfaces as an ANR. See #444 for the full description.
Change
Wrap the seven dispatch sites in
UiThreadUtil.runOnUiThread { … }so the emitting thread hands the event off instead of blocking inside the bridge call while holding Rive's lock:onPlay,onPause,onStop,onLoopEnd,onStateChanged,onRiveEventReceived, and thesendErrorToRNerror path.Mechanical change — one new import, no change to event names, payloads, or relative ordering. Delivery simply starts from the UI thread, which is also the usual expectation for
RCTEventEmitter.Testing — please read
I want to be straight about this rather than imply more verification than I did:
9.8.0in a React Native 0.86.2 / Expo 57 app on Fabric, and the ANR stopped.mainand have not compiledmainor run the example app locally, so please treat CI and your own review as the real gate. The diff is small enough to read in full, andmain's call sites are byte-identical to the ones we patch.I'm aware this is the legacy runtime and
@rive-app/react-nativeis where things are heading — offering this under the README's "medium term: address major concerns in this legacy package while supporting migration", since an ANR is painful for apps mid-migration. Entirely understand if you'd rather fix it differently or not at all.