From 745ff1086a98e2385fb0b34daa07c8f92d4c8e3e Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Thu, 21 May 2020 07:17:14 -0700 Subject: [PATCH] Prevent ABA problem during YogaLayoutableShadowNode cloning Summary: Changelog: [Internal] # Problem Yoga internally uses address of owner to determine whether a node should be cloned or it shouldn't. During layout, it traverses the tree and looks whether current parent is equal to child's owner. If it isn't it means the yoga node is shared between 2+ trees and need to be cloned before mutated. Parent in yoga is stored as reference to the parent. We can run into an issue where yoga node is shared between 2+ trees, but its current parent is equal to child's owner. This is because we reallocate new parent at address where its previous parent lived. This happens over few iterations, the old parent is first deallocated. This is known as ABA problem. # Solution When we are cloning node, we loop over all children to see whether any of the is not using address of new `yogaNode_` as its owner. At this point we know this is accidental and set its owner to `0xBADC0FFEE0DDF00D`. We chose `0xBADC0FFEE0DDF00D` because when someone is debugging this in LLDB and prints this address, it will hint them that it was artificially set and can string search for it in the codebase. Reviewed By: shergin Differential Revision: D21641096 fbshipit-source-id: c8b1b4487ea02b367f5831c1cdac055bce79c856 --- .../components/view/yoga/YogaLayoutableShadowNode.cpp | 9 +++++++++ .../components/view/yoga/YogaLayoutableShadowNode.h | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp index 9e15d72e04..1a9982b2cf 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp @@ -73,6 +73,7 @@ YogaLayoutableShadowNode::YogaLayoutableShadowNode( &initializeYogaConfig(yogaConfig_)) { yogaNode_.setContext(this); yogaNode_.setOwner(nullptr); + updateYogaChildrenOwnersIfNeeded(); // Yoga node must inherit dirty flag. assert( @@ -223,6 +224,14 @@ bool YogaLayoutableShadowNode::doesOwn( return child.yogaNode_.getOwner() == &yogaNode_; } +void YogaLayoutableShadowNode::updateYogaChildrenOwnersIfNeeded() { + for (auto &childYogaNode : yogaNode_.getChildren()) { + if (childYogaNode->getOwner() == &yogaNode_) { + childYogaNode->setOwner(reinterpret_cast(0xBADC0FFEE0DDF00D)); + } + } +} + void YogaLayoutableShadowNode::updateYogaChildren() { if (getTraits().check(ShadowNodeTraits::Trait::LeafYogaNode)) { return; diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h index e9ea29cd47..59148e333b 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h @@ -100,6 +100,16 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode { mutable YGNode yogaNode_; private: + /* + * Goes over `yogaNode_.getChildren()` and in case child's owner is + * equal to address of `yogaNode_`, it sets child's owner address + * to `0xBADC0FFEE0DDF00D`. This is magic constant, the intention + * is to make debugging easier when the address pops up in debugger. + * This prevents ABA problem where child yoga node goes from owned -> unowned + * -> back to owned because its parent is allocated at the same address. + */ + void updateYogaChildrenOwnersIfNeeded(); + /* * Return true if child's yogaNode's owner is this->yogaNode_. Otherwise * returns false.