Fix render callback start/stop race in C++ NativeAnimated

packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.cpp

@@ -551,54 +551,54 @@ 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);
   }
 }
 
 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);

packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.h

@@ -22,6 +22,7 @@
 #include <react/renderer/core/ReactPrimitives.h>
 #include <react/renderer/core/ShadowNode.h>
 #include <react/renderer/uimanager/UIManagerAnimationBackend.h>
+#include <atomic>
 #include <chrono>
 #include <memory>
 #include <mutex>
@@ -298,7 +299,9 @@ class NativeAnimatedNodesManager : public std::enable_shared_from_this<NativeAni
   bool warnedAboutGraphTraversal_ = false;
 #endif
 
-  CallbackId animationBackendCallbackId_{0};
+  // Render callback registered with the shared AnimationBackend (0 == none).
+  static constexpr CallbackId kRenderCallbackNotStarted = 0;
+  std::atomic<CallbackId> animationBackendCallbackId_{kRenderCallbackNotStarted};
 
   friend class ColorAnimatedNode;
   friend class AnimationDriver;

packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationBackend.h

@@ -80,7 +80,8 @@ class AnimationBackend : public UIManagerAnimationBackend {
   std::weak_ptr<UIManager> uiManager_;
   std::shared_ptr<CallInvoker> 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