Skip to content

Commit dd5c383

Browse files
zeyapmeta-codesync[bot]
authored andcommitted
Decide NativeAnimated backend per-instance instead of live flag (#57205)
Summary: Pull Request resolved: #57205 ## Changelog: [Internal][Fixed] - Fix EXC_BAD_ACCESS / KERN_INVALID_ADDRESS in C++ Animated when `useSharedAnimatedBackend()` flips across a reused runtime C++ Animated chose between the legacy and shared-`AnimationBackend` code paths by reading `ReactNativeFeatureFlags::useSharedAnimatedBackend()` live, in many places. That flag is a process-global singleton (`ReactNativeFeatureFlags::accessor_`). On some app the global is reset and re-applied on every user switch (`FBReactModule setUpReactNativeFeatureFlags` -> `dangerouslyReset()`/`override()`), and the previous user's runtime is kept alive and reused. The shared `AnimationBackend` is attached only once, when an instance's `Scheduler` is constructed, gated on the flag at that moment. On a multi-account device the global flag could therefore read true on a reused instance whose backend was never attached, so `getOrCreate` took the shared path and dereferenced a null backend. It also let JS and C++ disagree, since JS caches the flag per runtime while C++ followed the mutated global. Fix: make the per-instance decision once and use it everywhere instead of the live flag. - `NativeAnimatedNodesManagerProvider::getOrCreate` selects the path by whether the shared `AnimationBackend` actually exists for this instance (`unstable_getAnimationBackend().lock() != nullptr`). - `NativeAnimatedNodesManager` stores a `const bool useSharedAnimatedBackend_`, latched in its constructor (true for the shared-backend ctor, false for the legacy ctor), and exposes it via `useSharedAnimatedBackend()`. All internal reads now use the member. - `PropsAnimatedNode` reads the decision through `manager_->useSharedAnimatedBackend()`. The attach side (`Scheduler`) is intentionally unchanged: it remains the single construction-time read that latches the per-instance decision the rest of the code now follows. This keeps JS and C++ consistent across global flag flips and removes the null dereference. Reviewed By: sbuggay Differential Revision: D108428720 fbshipit-source-id: dff283cd3671866395d1b07b6c9c72e504aecea2
1 parent 066c0d8 commit dd5c383

13 files changed

Lines changed: 41 additions & 21 deletions

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ NativeAnimatedNodesManager::NativeAnimatedNodesManager(
7373
StartOnRenderCallback&& startOnRenderCallback,
7474
StopOnRenderCallback&& stopOnRenderCallback,
7575
FrameRateListenerCallback&& frameRateListenerCallback) noexcept
76-
: directManipulationCallback_(std::move(directManipulationCallback)),
76+
: useSharedAnimatedBackend_(false),
77+
directManipulationCallback_(std::move(directManipulationCallback)),
7778
fabricCommitCallback_(std::move(fabricCommitCallback)),
7879
resolvePlatformColor_(std::move(resolvePlatformColor)),
7980
startOnRenderCallback_(std::move(startOnRenderCallback)),
@@ -97,7 +98,7 @@ NativeAnimatedNodesManager::NativeAnimatedNodesManager(
9798

9899
NativeAnimatedNodesManager::NativeAnimatedNodesManager(
99100
std::shared_ptr<UIManagerAnimationBackend> animationBackend) noexcept
100-
: animationBackend_(animationBackend) {}
101+
: animationBackend_(animationBackend), useSharedAnimatedBackend_(true) {}
101102

102103
NativeAnimatedNodesManager::~NativeAnimatedNodesManager() noexcept {
103104
stopRenderCallbackIfNeeded(true);
@@ -256,7 +257,7 @@ void NativeAnimatedNodesManager::disconnectAnimatedNodeFromView(
256257
auto node = getAnimatedNode<PropsAnimatedNode>(propsNodeTag);
257258
if (node != nullptr) {
258259
node->disconnectFromView(viewTag);
259-
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
260+
if (useSharedAnimatedBackend_) {
260261
node->disconnectFromShadowNodeFamily();
261262
}
262263
{
@@ -521,7 +522,7 @@ void NativeAnimatedNodesManager::handleAnimatedEvent(
521522
// That's why, in case this is called from the UI thread, we need to
522523
// proactivelly trigger the animation loop to avoid showing stale
523524
// frames.
524-
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
525+
if (useSharedAnimatedBackend_) {
525526
if (auto animationBackend = animationBackend_.lock()) {
526527
animationBackend->pushAnimationMutations(
527528
[this](AnimationTimestamp timestamp) -> AnimationMutations {
@@ -561,7 +562,7 @@ void NativeAnimatedNodesManager::startRenderCallbackIfNeeded(bool isAsync) {
561562
return;
562563
}
563564

564-
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
565+
if (useSharedAnimatedBackend_) {
565566
if (auto animationBackend = animationBackend_.lock()) {
566567
auto weak = weak_from_this();
567568
animationBackendCallbackId_ = animationBackend->start(
@@ -589,7 +590,7 @@ void NativeAnimatedNodesManager::stopRenderCallbackIfNeeded(
589590
// stopRenderCallbackIfNeeded is always called from the UI thread.
590591
auto isRenderCallbackStarted = isRenderCallbackStarted_.exchange(false);
591592

592-
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
593+
if (useSharedAnimatedBackend_) {
593594
if (isRenderCallbackStarted) {
594595
if (auto animationBackend = animationBackend_.lock()) {
595596
animationBackend->stop(animationBackendCallbackId_);
@@ -922,7 +923,7 @@ void NativeAnimatedNodesManager::schedulePropsCommit(
922923
bool layoutStyleUpdated,
923924
bool forceFabricCommit,
924925
ShadowNodeFamily::Weak shadowNodeFamily) noexcept {
925-
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
926+
if (useSharedAnimatedBackend_) {
926927
if (forceFabricCommit) {
927928
shouldRequestAsyncFlush_.insert(viewTag);
928929
}
@@ -1029,7 +1030,7 @@ AnimationMutations NativeAnimatedNodesManager::onAnimationFrameForBackend(
10291030

10301031
AnimationMutations NativeAnimatedNodesManager::pullAnimationMutations(
10311032
AnimationTimestamp timestamp) {
1032-
if (!ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
1033+
if (!useSharedAnimatedBackend_) {
10331034
return {};
10341035
}
10351036
TraceSection s(
@@ -1119,7 +1120,7 @@ void NativeAnimatedNodesManager::flushAnimatedNodesCreatedAsync() noexcept {
11191120
}
11201121

11211122
void NativeAnimatedNodesManager::onRender() {
1122-
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
1123+
if (useSharedAnimatedBackend_) {
11231124
return;
11241125
}
11251126
TraceSection s(

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ class NativeAnimatedNodesManager : public std::enable_shared_from_this<NativeAni
7878
NativeAnimatedNodesManager(NativeAnimatedNodesManager &&) = delete;
7979
NativeAnimatedNodesManager &operator=(NativeAnimatedNodesManager &&) = delete;
8080

81+
// Whether this instance was constructed to use the shared AnimationBackend.
82+
// Latched at construction, so it is immune to later flips of the
83+
// process-global useSharedAnimatedBackend() flag.
84+
bool useSharedAnimatedBackend() const noexcept
85+
{
86+
return useSharedAnimatedBackend_;
87+
}
88+
8189
template <typename T, typename = std::enable_if_t<std::is_base_of_v<AnimatedNode, T>>>
8290
T *getAnimatedNode(Tag tag) const
8391
requires(std::is_base_of_v<AnimatedNode, T>)
@@ -219,6 +227,12 @@ class NativeAnimatedNodesManager : public std::enable_shared_from_this<NativeAni
219227

220228
std::weak_ptr<UIManagerAnimationBackend> animationBackend_;
221229

230+
// Latched per-instance copy of which backend this manager uses, set from the
231+
// constructor that ran (true for the shared-AnimationBackend ctor). Reads stay
232+
// stable even when the global useSharedAnimatedBackend() flag is re-overridden
233+
// on another RN runtime.
234+
const bool useSharedAnimatedBackend_;
235+
222236
std::unique_ptr<AnimatedNode> animatedNode(Tag tag, const folly::dynamic &config) noexcept;
223237

224238
static thread_local bool isOnRenderThread_;

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include "NativeAnimatedNodesManagerProvider.h"
99

1010
#include <glog/logging.h>
11-
#include <react/featureflags/ReactNativeFeatureFlags.h>
1211
#include <react/renderer/animated/MergedValueDispatcher.h>
1312
#include <react/renderer/animated/internal/AnimatedMountingOverrideDelegate.h>
1413
#include <react/renderer/animated/internal/primitives.h>
@@ -51,8 +50,9 @@ NativeAnimatedNodesManagerProvider::getOrCreate(
5150

5251
auto* uiManager = &UIManagerBinding::getBinding(runtime)->getUIManager();
5352

54-
if (!ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
55-
// === PATH 1: Legacy Backend (useSharedAnimatedBackend = false) ===
53+
auto animationBackend = uiManager->unstable_getAnimationBackend().lock();
54+
if (animationBackend == nullptr) {
55+
// === PATH 1: Legacy Backend (no shared AnimationBackend attached) ===
5656
// Uses the architecture with MergedValueDispatcher and
5757
// AnimatedMountingOverrideDelegate
5858

@@ -130,13 +130,10 @@ NativeAnimatedNodesManagerProvider::getOrCreate(
130130

131131
uiManager->setNativeAnimatedDelegate(nativeAnimatedDelegate_);
132132
} else {
133-
// === PATH 2: Shared AnimationBackend (useSharedAnimatedBackend = true) ===
133+
// === PATH 2: Shared AnimationBackend ===
134134
// Uses the shared AnimationBackend from UIManager. The backend handles all
135-
// animation commits and platform integration internally.
136-
137-
auto animationBackend = uiManager->unstable_getAnimationBackend().lock();
138-
react_native_assert(
139-
animationBackend != nullptr && "animationBackend is nullptr");
135+
// animation commits and platform integration internally. It is guaranteed
136+
// non-null here because it was successfully locked above.
140137
animationBackend->registerJSInvoker(jsInvoker);
141138

142139
nativeAnimatedNodesManager_ =

packages/react-native/ReactCommon/react/renderer/animated/nodes/PropsAnimatedNode.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
#include "PropsAnimatedNode.h"
1313

1414
#include <react/debug/react_native_assert.h>
15-
#include <react/featureflags/ReactNativeFeatureFlags.h>
1615
#include <react/renderer/animated/NativeAnimatedNodesManager.h>
1716
#include <react/renderer/animated/nodes/ColorAnimatedNode.h>
1817
#include <react/renderer/animated/nodes/ObjectAnimatedNode.h>
@@ -84,7 +83,7 @@ void PropsAnimatedNode::disconnectFromView(Tag viewTag) {
8483
void PropsAnimatedNode::restoreDefaultValues() {
8584
// If node is already disconnected from View, we cannot restore default values
8685
if (connectedViewTag_ != animated::undefinedAnimatedNodeIdentifier) {
87-
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
86+
if (manager_->useSharedAnimatedBackend()) {
8887
manager_->schedulePropsCommit(
8988
connectedViewTag_,
9089
folly::dynamic::object(),
@@ -166,7 +165,7 @@ void PropsAnimatedNode::update(bool forceFabricCommit) {
166165

167166
layoutStyleUpdated_ = isLayoutStyleUpdated(getConfig()["props"], *manager_);
168167

169-
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
168+
if (manager_->useSharedAnimatedBackend()) {
170169
manager_->schedulePropsCommit(
171170
connectedViewTag_,
172171
props_,

scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3491,6 +3491,7 @@ class facebook::react::NativeAnimatedNodesManager : public std::enable_shared_fr
34913491
public bool commitProps();
34923492
public bool hasManagedProps() const noexcept;
34933493
public bool isOnRenderThread() const noexcept;
3494+
public bool useSharedAnimatedBackend() const noexcept;
34943495
public facebook::react::AnimationMutations onAnimationFrameForBackend(facebook::react::AnimatedPropsBuilder& propsBuilder, facebook::react::AnimationTimestamp timestamp);
34953496
public facebook::react::AnimationMutations pullAnimationMutations(facebook::react::AnimationTimestamp timestamp);
34963497
public facebook::react::NativeAnimatedNodesManager& operator=(const facebook::react::NativeAnimatedNodesManager&) = delete;

scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3375,6 +3375,7 @@ class facebook::react::NativeAnimatedNodesManager : public std::enable_shared_fr
33753375
public bool commitProps();
33763376
public bool hasManagedProps() const noexcept;
33773377
public bool isOnRenderThread() const noexcept;
3378+
public bool useSharedAnimatedBackend() const noexcept;
33783379
public facebook::react::AnimationMutations onAnimationFrameForBackend(facebook::react::AnimatedPropsBuilder& propsBuilder, facebook::react::AnimationTimestamp timestamp);
33793380
public facebook::react::AnimationMutations pullAnimationMutations(facebook::react::AnimationTimestamp timestamp);
33803381
public facebook::react::NativeAnimatedNodesManager& operator=(const facebook::react::NativeAnimatedNodesManager&) = delete;

scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3488,6 +3488,7 @@ class facebook::react::NativeAnimatedNodesManager : public std::enable_shared_fr
34883488
public bool commitProps();
34893489
public bool hasManagedProps() const noexcept;
34903490
public bool isOnRenderThread() const noexcept;
3491+
public bool useSharedAnimatedBackend() const noexcept;
34913492
public facebook::react::AnimationMutations onAnimationFrameForBackend(facebook::react::AnimatedPropsBuilder& propsBuilder, facebook::react::AnimationTimestamp timestamp);
34923493
public facebook::react::AnimationMutations pullAnimationMutations(facebook::react::AnimationTimestamp timestamp);
34933494
public facebook::react::NativeAnimatedNodesManager& operator=(const facebook::react::NativeAnimatedNodesManager&) = delete;

scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5710,6 +5710,7 @@ class facebook::react::NativeAnimatedNodesManager : public std::enable_shared_fr
57105710
public bool commitProps();
57115711
public bool hasManagedProps() const noexcept;
57125712
public bool isOnRenderThread() const noexcept;
5713+
public bool useSharedAnimatedBackend() const noexcept;
57135714
public facebook::react::AnimationMutations onAnimationFrameForBackend(facebook::react::AnimatedPropsBuilder& propsBuilder, facebook::react::AnimationTimestamp timestamp);
57145715
public facebook::react::AnimationMutations pullAnimationMutations(facebook::react::AnimationTimestamp timestamp);
57155716
public facebook::react::NativeAnimatedNodesManager& operator=(const facebook::react::NativeAnimatedNodesManager&) = delete;

scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5622,6 +5622,7 @@ class facebook::react::NativeAnimatedNodesManager : public std::enable_shared_fr
56225622
public bool commitProps();
56235623
public bool hasManagedProps() const noexcept;
56245624
public bool isOnRenderThread() const noexcept;
5625+
public bool useSharedAnimatedBackend() const noexcept;
56255626
public facebook::react::AnimationMutations onAnimationFrameForBackend(facebook::react::AnimatedPropsBuilder& propsBuilder, facebook::react::AnimationTimestamp timestamp);
56265627
public facebook::react::AnimationMutations pullAnimationMutations(facebook::react::AnimationTimestamp timestamp);
56275628
public facebook::react::NativeAnimatedNodesManager& operator=(const facebook::react::NativeAnimatedNodesManager&) = delete;

scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5707,6 +5707,7 @@ class facebook::react::NativeAnimatedNodesManager : public std::enable_shared_fr
57075707
public bool commitProps();
57085708
public bool hasManagedProps() const noexcept;
57095709
public bool isOnRenderThread() const noexcept;
5710+
public bool useSharedAnimatedBackend() const noexcept;
57105711
public facebook::react::AnimationMutations onAnimationFrameForBackend(facebook::react::AnimatedPropsBuilder& propsBuilder, facebook::react::AnimationTimestamp timestamp);
57115712
public facebook::react::AnimationMutations pullAnimationMutations(facebook::react::AnimationTimestamp timestamp);
57125713
public facebook::react::NativeAnimatedNodesManager& operator=(const facebook::react::NativeAnimatedNodesManager&) = delete;

0 commit comments

Comments
 (0)