Skip to content

Commit

Permalink
Avoid shared_mutex re-entrancy for setNativeProps_DEPRECATED
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rozele authored and facebook-github-bot committed Apr 2, 2024
1 parent 0c02b26 commit f00ac16
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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(
Expand All @@ -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});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down

0 comments on commit f00ac16

Please sign in to comment.