From 7de94ed6a9048869f0a22b10d3b0b65f03d1cfbc Mon Sep 17 00:00:00 2001 From: Bartlomiej Bloniarz Date: Thu, 18 Jun 2026 03:58:48 -0700 Subject: [PATCH] Fix render callback start/stop race in C++ NativeAnimated Summary: The C++ NativeAnimated manager coordinated its render-callback start/stop through an atomic flag, but registered/unregistered the callback and tracked its id outside that atomic. When start (JS thread) and stop (UI thread) ran concurrently, a stop could act on a stale id and miss the just-registered callback, leaving it orphaned. Orphaned callbacks are never removed and run on every frame, accumulating over time. Coordinate the shared animation backend path through a single atomic callback id (0 means "not started"): start registers then publishes the id with a compare-exchange, undoing the registration if it lost the race; stop atomically takes the id and stops exactly that callback. Reserve backend callback id 0 so it can serve as the sentinel. Changelog: [General][Fixed] - Fix a race in the C++ Animated render-callback lifecycle that could leave stale callbacks running on every frame Differential Revision: D109009181 --- .../animated/NativeAnimatedNodesManager.cpp | 58 +++++++++---------- .../animated/NativeAnimatedNodesManager.h | 5 +- .../animationbackend/AnimationBackend.h | 3 +- 3 files changed, 35 insertions(+), 31 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.cpp b/packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.cpp index 363c49a0cba..3415b8a421c 100644 --- a/packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.cpp +++ b/packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.cpp @@ -551,32 +551,35 @@ NativeAnimatedNodesManager::ensureEventEmitterListener() noexcept { } void NativeAnimatedNodesManager::startRenderCallbackIfNeeded(bool isAsync) { - // This method can be called from either the UI thread or JavaScript thread. - // It ensures `startOnRenderCallback_` is called exactly once using atomic - // operations. We use std::atomic_bool rather than std::mutex to avoid - // potential deadlocks that could occur if we called external code while - // holding a mutex. - auto isRenderCallbackStarted = isRenderCallbackStarted_.exchange(true); - if (isRenderCallbackStarted) { - // onRender callback is already started. - return; - } - + // Called from either the UI thread or the JavaScript thread. if (useSharedAnimatedBackend_) { - if (auto animationBackend = animationBackend_.lock()) { - auto weak = weak_from_this(); - animationBackendCallbackId_ = animationBackend->start( - [weak](AnimationTimestamp timestamp) -> AnimationMutations { - if (auto self = weak.lock()) { - return self->pullAnimationMutations(timestamp); - } - return {}; - }); + if (animationBackendCallbackId_.load() != kRenderCallbackNotStarted) { + return; + } + auto animationBackend = animationBackend_.lock(); + if (!animationBackend) { + return; + } + auto weak = weak_from_this(); + auto callbackId = animationBackend->start( + [weak](AnimationTimestamp timestamp) -> AnimationMutations { + if (auto self = weak.lock()) { + return self->pullAnimationMutations(timestamp); + } + return {}; + }); + auto expected = kRenderCallbackNotStarted; + if (!animationBackendCallbackId_.compare_exchange_strong( + expected, callbackId)) { + animationBackend->stop(callbackId); } - return; } + auto isRenderCallbackStarted = isRenderCallbackStarted_.exchange(true); + if (isRenderCallbackStarted) { + return; + } if (startOnRenderCallback_) { startOnRenderCallback_([this]() { onRender(); }, isAsync); } @@ -584,21 +587,18 @@ void NativeAnimatedNodesManager::startRenderCallbackIfNeeded(bool isAsync) { void NativeAnimatedNodesManager::stopRenderCallbackIfNeeded( bool isAsync) noexcept { - // When multiple threads reach this point, only one thread should call - // stopOnRenderCallback_. This synchronization is primarily needed during - // destruction of NativeAnimatedNodesManager. In normal operation, - // stopRenderCallbackIfNeeded is always called from the UI thread. - auto isRenderCallbackStarted = isRenderCallbackStarted_.exchange(false); - if (useSharedAnimatedBackend_) { - if (isRenderCallbackStarted) { + auto callbackId = + animationBackendCallbackId_.exchange(kRenderCallbackNotStarted); + if (callbackId != kRenderCallbackNotStarted) { if (auto animationBackend = animationBackend_.lock()) { - animationBackend->stop(animationBackendCallbackId_); + animationBackend->stop(callbackId); } } return; } + auto isRenderCallbackStarted = isRenderCallbackStarted_.exchange(false); if (isRenderCallbackStarted) { if (stopOnRenderCallback_) { stopOnRenderCallback_(isAsync); diff --git a/packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.h b/packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.h index 26a8209a61d..f3a9408ac98 100644 --- a/packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.h +++ b/packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -298,7 +299,9 @@ class NativeAnimatedNodesManager : public std::enable_shared_from_this animationBackendCallbackId_{kRenderCallbackNotStarted}; friend class ColorAnimatedNode; friend class AnimationDriver; diff --git a/packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationBackend.h b/packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationBackend.h index c01ff23a8e7..5228abaadba 100644 --- a/packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationBackend.h +++ b/packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationBackend.h @@ -80,7 +80,8 @@ class AnimationBackend : public UIManagerAnimationBackend { std::weak_ptr uiManager_; std::shared_ptr jsInvoker_; bool isRenderCallbackStarted_{false}; - CallbackId nextCallbackId_{0}; + // Starts at 1 so 0 can be used as a "no callback" sentinel by callers. + CallbackId nextCallbackId_{1}; std::mutex mutex_; }; } // namespace facebook::react