From 2944752a5de8eba3b240d699baedda83592b9504 Mon Sep 17 00:00:00 2001 From: Nick Lefever Date: Wed, 20 Nov 2024 00:37:21 -0800 Subject: [PATCH] Enable RSNRS only on JS thread (#47637) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/47637 Changelog: [Internal] RSNRS does not support off JS thread layout due to current fiber tree corruption when syncing happens on more than one shadow tree at the same time. This diff guarantees that RSNRS will only be enabled on the JS thread, avoiding any state corruption. Reviewed By: sammy-SC Differential Revision: D64500893 fbshipit-source-id: aa20f54a0fcfa47534ae099e95307a692bd9fd0f --- .../react/renderer/core/ShadowNode.cpp | 16 +++++++++++++++- .../ReactCommon/react/renderer/core/ShadowNode.h | 2 ++ .../react/renderer/core/tests/ShadowNodeTest.cpp | 2 ++ .../react/runtime/React-RuntimeCore.podspec | 2 ++ .../ReactCommon/react/runtime/ReactInstance.cpp | 2 ++ 5 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp index 62827028223..a9ca4d71c9d 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp @@ -20,6 +20,19 @@ namespace facebook::react { +/* + * Runtime shadow node reference updates should only run from one thread at all + * times to avoid having more than one shadow tree updating the current fiber + * tree simultaneously. This thread_local flag allows enabling the updates for + * choses threads. + */ +thread_local bool useRuntimeShadowNodeReferenceUpdateOnThread{false}; // NOLINT + +/* static */ void ShadowNode::setUseRuntimeShadowNodeReferenceUpdateOnThread( + bool isEnabled) { + useRuntimeShadowNodeReferenceUpdateOnThread = isEnabled; +} + ShadowNode::SharedListOfShared ShadowNode::emptySharedShadowNodeSharedList() { static const auto emptySharedShadowNodeSharedList = std::make_shared(); @@ -308,7 +321,8 @@ void ShadowNode::transferRuntimeShadowNodeReference( void ShadowNode::transferRuntimeShadowNodeReference( const Shared& destinationShadowNode, const ShadowNodeFragment& fragment) const { - if (fragment.runtimeShadowNodeReference && + if (useRuntimeShadowNodeReferenceUpdateOnThread && + fragment.runtimeShadowNodeReference && ReactNativeFeatureFlags::useRuntimeShadowNodeReferenceUpdate()) { transferRuntimeShadowNodeReference(destinationShadowNode); } diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h index 30f5ea8a6d2..7a3ad9cbc1b 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h @@ -61,6 +61,8 @@ class ShadowNode : public Sealable, return ShadowNodeTraits{}; } + static void setUseRuntimeShadowNodeReferenceUpdateOnThread(bool isEnabled); + #pragma mark - Constructors /* diff --git a/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp b/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp index ec962d0e144..7e6291973a9 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/tests/ShadowNodeTest.cpp @@ -154,6 +154,8 @@ class ShadowNodeTest : public testing::TestWithParam { } void SetUp() override { + ShadowNode::setUseRuntimeShadowNodeReferenceUpdateOnThread(true); + ReactNativeFeatureFlags::override( std::make_unique(GetParam())); } diff --git a/packages/react-native/ReactCommon/react/runtime/React-RuntimeCore.podspec b/packages/react-native/ReactCommon/react/runtime/React-RuntimeCore.podspec index 7d1a6b3b47b..eacfb1808dc 100644 --- a/packages/react-native/ReactCommon/react/runtime/React-RuntimeCore.podspec +++ b/packages/react-native/ReactCommon/react/runtime/React-RuntimeCore.podspec @@ -58,6 +58,8 @@ Pod::Spec.new do |s| s.dependency "React-runtimescheduler" s.dependency "React-utils" s.dependency "React-featureflags" + + add_dependency(s, "React-Fabric") if ENV["USE_HERMES"] == nil || ENV["USE_HERMES"] == "1" s.dependency "hermes-engine" diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index f6c07d7ff79..f24a006e697 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -92,6 +93,7 @@ ReactInstance::ReactInstance( jsi::Runtime& jsiRuntime = runtime->getRuntime(); SystraceSection s("ReactInstance::_runtimeExecutor[Callback]"); try { + ShadowNode::setUseRuntimeShadowNodeReferenceUpdateOnThread(true); callback(jsiRuntime); // If we have first-class support for microtasks,