Skip to content

Commit 95f172e

Browse files
coadofacebook-github-bot
authored andcommitted
Fix AnimationBackend choreographer deadlock with synchronous VirtualView events
Summary: When the shared animation backend is enabled, starting/stopping a native animation calls `AnimationBackendChoreographer.resume()/pause()` on the thread that started the animation, which can be the **JS thread**. The `resume()` method posted a frame callback to ReactChoreographer, acquiring the choreographer's callback queue. The UI thread might simultaneously hold that lock while dispatching a synchronous event through `executeSynchronouslyOnSameThread_CAN_DEADLOCK`. That path is chose after invoking the VirtualView `Visible` mode-change event. The `executeSynchronouslyOnSameThread_CAN_DEADLOCK` blocks main thread to access `jsi::Runtime` from the JS thread while holding a lock on the callbacks queue. There is a chance that resuming animation and the synchronous event will collide, as the first one tries to post a callback to the queue from the JS thread, waiting for release while the other holds a lock on that queue and waits for the JS thread to pass the runtime, causing a deadlock. The fix is to keep the choreographer frame callback permanently registered. It is posted once on the UI thread and re-posts itself every frame, running as a no-op while paused. The `resume` and `pause` methods now only toggle so accessing the lock on the callback queue is not necessary. Changelog: [Android][Fixed] - Fix deadlock between the UI and JS threads when native animations start while a synchronous VirtualView mode-change event is dispatched Differential Revision: D107378627
1 parent 64c9663 commit 95f172e

1 file changed

Lines changed: 27 additions & 32 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/AnimationBackendChoreographer.kt

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ package com.facebook.react.fabric
99

1010
import com.facebook.proguard.annotations.DoNotStripAny
1111
import com.facebook.react.bridge.ReactApplicationContext
12+
import com.facebook.react.bridge.UiThreadUtil
1213
import com.facebook.react.modules.core.ReactChoreographer
1314
import com.facebook.react.uimanager.GuardedFrameCallback
1415
import java.util.concurrent.atomic.AtomicBoolean
15-
import kotlin.synchronized
1616

1717
internal fun interface AnimationFrameCallback {
1818
fun onAnimationFrame(frameTimeMs: Double)
@@ -32,54 +32,49 @@ internal class AnimationBackendChoreographer(
3232
executeFrameCallback(frameTimeNanos)
3333
}
3434
}
35-
private val callbackPosted: AtomicBoolean = AtomicBoolean()
35+
36+
// When true, the always-registered frame callback runs as a no-op.
37+
//
38+
// The callback is registered with ReactChoreographer once (on the UI thread)
39+
// and re-posts itself every frame regardless of this flag, so it stays
40+
// registered for the lifetime of the choreographer.
3641
private val paused: AtomicBoolean = AtomicBoolean(true)
3742

38-
/*
39-
* resume() and pause() should be called with the same lock to avoid race conditions.
40-
*/
43+
init {
44+
// Register the self-reposting callback once, on the UI thread, so the
45+
// callback queues are only ever mutated from the UI thread.
46+
UiThreadUtil.runOnUiThread { postCallback() }
47+
}
4148

4249
fun resume() {
43-
if (paused.getAndSet(false)) {
44-
scheduleCallback()
45-
}
50+
paused.set(false)
4651
}
4752

4853
fun pause() {
49-
val shouldRemove: Boolean
50-
synchronized(paused) {
51-
shouldRemove = !paused.getAndSet(true) && callbackPosted.getAndSet(false)
52-
}
53-
if (shouldRemove) {
54-
reactChoreographer.removeFrameCallback(
55-
ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE,
56-
choreographerCallback,
57-
)
58-
}
54+
paused.set(true)
5955
}
6056

61-
private fun scheduleCallback() {
62-
val shouldPost: Boolean
63-
synchronized(paused) { shouldPost = !paused.get() && !callbackPosted.getAndSet(true) }
64-
if (shouldPost) {
65-
reactChoreographer.postFrameCallback(
66-
ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE,
67-
choreographerCallback,
68-
)
69-
}
57+
private fun postCallback() {
58+
reactChoreographer.postFrameCallback(
59+
ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE,
60+
choreographerCallback,
61+
)
7062
}
7163

7264
private fun executeFrameCallback(frameTimeNanos: Long) {
73-
callbackPosted.set(false)
7465
val currentFrameTimeMs = calculateTimestamp(frameTimeNanos)
75-
// It is possible for ChoreographerCallback to be executed twice within the same frame
76-
// due to frame drops. If this occurs, the additional callback execution should be ignored.
77-
if (currentFrameTimeMs > lastFrameTimeMs) {
66+
// Only drive the animation backend while enabled. It is possible for the
67+
// ChoreographerCallback to be executed twice within the same frame due to
68+
// frame drops; if this occurs, the additional callback execution should be
69+
// ignored.
70+
if (!paused.get() && currentFrameTimeMs > lastFrameTimeMs) {
7871
frameCallback?.onAnimationFrame(currentFrameTimeMs)
7972
}
8073

8174
lastFrameTimeMs = currentFrameTimeMs
82-
scheduleCallback()
75+
// Always re-post (on the UI thread) so the callback stays registered for the
76+
// next frame, whether or not we are currently paused.
77+
postCallback()
8378
}
8479

8580
private fun calculateTimestamp(frameTimeNanos: Long): Double {

0 commit comments

Comments
 (0)