Skip to content

Commit 22d3275

Browse files
Bartlomiej Bloniarzmeta-codesync[bot]
authored andcommitted
Fix deadlock in AnimationBackendChoreographer
Summary: scheduleCallback() and pause() held synchronized(paused) while calling ReactChoreographer.postFrameCallback/removeFrameCallback, which acquire synchronized(callbackQueues). ReactChoreographer dispatches frame callbacks inside synchronized(callbackQueues), invoking executeFrameCallback -> scheduleCallback -> synchronized(paused). This is a lock ordering inversion: background thread acquires paused then callbackQueues, main thread acquires callbackQueues then paused. Fix: move postFrameCallback/removeFrameCallback calls outside the synchronized(paused) block. The atomic state check (paused + callbackPosted) stays inside the lock, only the ReactChoreographer interaction moves outside. Why this is safe: - callbackPosted.getAndSet(true) inside the lock guarantees at most one thread proceeds to post. A concurrent scheduleCallback will see callbackPosted=true and bail out. - If pause() runs between the lock release and postFrameCallback: pause sets paused=true and callbackPosted=false, then calls removeFrameCallback. Two outcomes depending on ordering inside callbackQueues (which serializes both calls): (a) post runs first, then remove cancels it — clean. (b) remove runs first (nothing to remove), then post adds a callback. That callback fires once, executeFrameCallback sees paused=true via scheduleCallback and does not re-post. Net effect: one extra no-op frame, then the loop stops. - resume() already operated without synchronized(paused) before this change, so no new races are introduced on that path. ## Changelog: [Android] [Changed] - AnimationBackendChoreographer doesn't guard the ReactChoreographer post/remove with synchronized(paused) Reviewed By: zeyap Differential Revision: D99099455 fbshipit-source-id: 5c5e9e76ec23fd300a0a67c98b70a9c5bdb08f30
1 parent bf277cb commit 22d3275

File tree

1 file changed

+15
-13
lines changed

1 file changed

+15
-13
lines changed

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

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,24 +46,26 @@ internal class AnimationBackendChoreographer(
4646
}
4747

4848
fun pause() {
49+
val shouldRemove: Boolean
4950
synchronized(paused) {
50-
if (!paused.getAndSet(true) && callbackPosted.getAndSet(false)) {
51-
reactChoreographer.removeFrameCallback(
52-
ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE,
53-
choreographerCallback,
54-
)
55-
}
51+
shouldRemove = !paused.getAndSet(true) && callbackPosted.getAndSet(false)
52+
}
53+
if (shouldRemove) {
54+
reactChoreographer.removeFrameCallback(
55+
ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE,
56+
choreographerCallback,
57+
)
5658
}
5759
}
5860

5961
private fun scheduleCallback() {
60-
synchronized(paused) {
61-
if (!paused.get() && !callbackPosted.getAndSet(true)) {
62-
reactChoreographer.postFrameCallback(
63-
ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE,
64-
choreographerCallback,
65-
)
66-
}
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+
)
6769
}
6870
}
6971

0 commit comments

Comments
 (0)