From f00ac1604bf2147594815154d9e28e0ff5aad0fa Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Tue, 2 Apr 2024 08:04:02 -0700 Subject: [PATCH] Avoid shared_mutex re-entrancy for setNativeProps_DEPRECATED Summary: It's best to avoid mutex reentrancy, even when using "read" locks (i.e., std::shared_lock). Since we already have a read lock on the ShadowTree in `UIManager::setNativeProps_DEPRECATED`, we can avoid the reentrancy by grabbing ancestor node from the shadow tree instead of relying on a recursive call to ShadowTreeRegistry::visit ## Changelog [GENERAL][FIXED] - Avoid ShadowTreeRegistry::mutex_ read lock reentrancy Differential Revision: D55640201 --- .../react/renderer/uimanager/UIManager.cpp | 11 ++++++++++- .../ReactCommon/react/renderer/uimanager/UIManager.h | 4 ++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp index 6e909a73902b2f..e5ced223b61324 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp @@ -268,7 +268,12 @@ ShadowNode::Shared UIManager::getNewestCloneOfShadowNode( shadowNode.getSurfaceId(), [&](const ShadowTree& shadowTree) { ancestorShadowNode = shadowTree.getCurrentRevision().rootShadowNode; }); + return getNewestCloneOfShadowNodeFromAncestor(shadowNode, ancestorShadowNode); +} +ShadowNode::Shared UIManager::getNewestCloneOfShadowNodeFromAncestor( + const ShadowNode& shadowNode, + const ShadowNode::Shared& ancestorShadowNode) const { if (!ancestorShadowNode) { return nullptr; } @@ -427,6 +432,8 @@ void UIManager::setNativeProps_DEPRECATED( family.getSurfaceId(), [&](const ShadowTree& shadowTree) { // The lambda passed to `commit` may be executed multiple times. // We need to create fresh copy of the `RawProps` object each time. + auto ancestorShadowNode = + shadowTree.getCurrentRevision().rootShadowNode; shadowTree.commit( [&](RootShadowNode const& oldRootShadowNode) { auto rootNode = oldRootShadowNode.cloneTree( @@ -438,7 +445,9 @@ void UIManager::setNativeProps_DEPRECATED( family.getSurfaceId(), *contextContainer_.get()}; auto props = componentDescriptor.cloneProps( propsParserContext, - getNewestCloneOfShadowNode(*shadowNode)->getProps(), + getNewestCloneOfShadowNodeFromAncestor( + *shadowNode, ancestorShadowNode) + ->getProps(), RawProps(rawProps)); return oldShadowNode.clone({/* .props = */ props}); diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.h b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.h index d32dca94b19081..e0dc7e98a485aa 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.h +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.h @@ -218,6 +218,10 @@ class UIManager final : public ShadowTreeDelegate { const jsi::Value& successCallback, const jsi::Value& failureCallback) const; + ShadowNode::Shared getNewestCloneOfShadowNodeFromAncestor( + const ShadowNode& shadowNode, + const ShadowNode::Shared& ancestorShadowNode) const; + SharedComponentDescriptorRegistry componentDescriptorRegistry_; UIManagerDelegate* delegate_{}; UIManagerAnimationDelegate* animationDelegate_{nullptr};