diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/consistency/LazyShadowTreeRevisionConsistencyManager.cpp b/packages/react-native/ReactCommon/react/renderer/uimanager/consistency/LazyShadowTreeRevisionConsistencyManager.cpp index 53e91ce7b95..b3f98e26f50 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/consistency/LazyShadowTreeRevisionConsistencyManager.cpp +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/consistency/LazyShadowTreeRevisionConsistencyManager.cpp @@ -18,6 +18,7 @@ LazyShadowTreeRevisionConsistencyManager:: void LazyShadowTreeRevisionConsistencyManager::updateCurrentRevision( SurfaceId surfaceId, RootShadowNode::Shared rootShadowNode) { + std::unique_lock lock(capturedRootShadowNodesForConsistencyMutex_); capturedRootShadowNodesForConsistency_[surfaceId] = std::move(rootShadowNode); } @@ -26,18 +27,28 @@ void LazyShadowTreeRevisionConsistencyManager::updateCurrentRevision( RootShadowNode::Shared LazyShadowTreeRevisionConsistencyManager::getCurrentRevision( SurfaceId surfaceId) { - auto it = capturedRootShadowNodesForConsistency_.find(surfaceId); - if (it != capturedRootShadowNodesForConsistency_.end()) { - return it->second; + { + std::unique_lock lock(capturedRootShadowNodesForConsistencyMutex_); + + auto it = capturedRootShadowNodesForConsistency_.find(surfaceId); + if (it != capturedRootShadowNodesForConsistency_.end()) { + return it->second; + } } + // This method is only going to be called from JS, so we don't need to protect + // the access to the shadow tree registry as well. + // If this was multi-threaded, we would need to protect it to avoid capturing + // root shadow nodes concurrently. RootShadowNode::Shared rootShadowNode; - shadowTreeRegistry_.visit(surfaceId, [&](const ShadowTree& shadowTree) { rootShadowNode = shadowTree.getCurrentRevision().rootShadowNode; }); - capturedRootShadowNodesForConsistency_[surfaceId] = rootShadowNode; + { + std::unique_lock lock(capturedRootShadowNodesForConsistencyMutex_); + capturedRootShadowNodesForConsistency_[surfaceId] = rootShadowNode; + } return rootShadowNode; } @@ -65,6 +76,8 @@ void LazyShadowTreeRevisionConsistencyManager::unlockRevisions() { } isLocked_ = false; + + std::unique_lock lock(capturedRootShadowNodesForConsistencyMutex_); capturedRootShadowNodesForConsistency_.clear(); } diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/consistency/LazyShadowTreeRevisionConsistencyManager.h b/packages/react-native/ReactCommon/react/renderer/uimanager/consistency/LazyShadowTreeRevisionConsistencyManager.h index d507263cce5..3de32c12e84 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/consistency/LazyShadowTreeRevisionConsistencyManager.h +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/consistency/LazyShadowTreeRevisionConsistencyManager.h @@ -12,6 +12,7 @@ #include #include #include +#include namespace facebook::react { @@ -42,6 +43,7 @@ class LazyShadowTreeRevisionConsistencyManager void unlockRevisions() override; private: + std::mutex capturedRootShadowNodesForConsistencyMutex_; std::unordered_map capturedRootShadowNodesForConsistency_; ShadowTreeRegistry& shadowTreeRegistry_; diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/consistency/tests/LazyShadowTreeRevisionConsistencyManagerTest.cpp b/packages/react-native/ReactCommon/react/renderer/uimanager/consistency/tests/LazyShadowTreeRevisionConsistencyManagerTest.cpp index ad7aabba4bb..fac644047c4 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/consistency/tests/LazyShadowTreeRevisionConsistencyManagerTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/consistency/tests/LazyShadowTreeRevisionConsistencyManagerTest.cpp @@ -301,4 +301,33 @@ TEST_F(LazyShadowTreeRevisionConsistencyManagerTest, testLockAfterUnlock) { consistencyManager_.unlockRevisions(); } +TEST_F(LazyShadowTreeRevisionConsistencyManagerTest, testUpdateToUnmounted) { + shadowTreeRegistry_.add(createShadowTree(0)); + + auto element = Element(); + auto builder = simpleComponentBuilder(); + auto newRootShadowNode = builder.build(element); + + shadowTreeRegistry_.visit( + 0, [newRootShadowNode](const ShadowTree& shadowTree) { + shadowTree.commit( + [&](const RootShadowNode& /*oldRootShadowNode*/) { + return newRootShadowNode; + }, + {}); + }); + + consistencyManager_.lockRevisions(); + + EXPECT_EQ( + consistencyManager_.getCurrentRevision(0).get(), newRootShadowNode.get()); + + consistencyManager_.updateCurrentRevision(0, nullptr); + + // Updated + EXPECT_EQ(consistencyManager_.getCurrentRevision(0).get(), nullptr); + + consistencyManager_.unlockRevisions(); +} + } // namespace facebook::react