From 01af828d16c6c57d612ee2902cfa906ceef4a67a Mon Sep 17 00:00:00 2001
From: Valentin Shergin <shergin@fb.com>
Date: Tue, 9 Oct 2018 16:25:10 -0700
Subject: [PATCH] Fabric: Fixed possible race condition in
 `ShadowTree::complete`

Summary: Quite obviously, having a `complete` method which accepts only `newRootShadowNode` was a baaad idea. When we `complete` or `commit` we always have to have two nodes (before and after). And only after layout and right before swapping (and acquiring the mutex) we have to verify that *current* root node is still the same as it was when we initialized the transaction (if not, we have to abort).

Reviewed By: mdvacca

Differential Revision: D10201902

fbshipit-source-id: 15adc78c5d31d6fd39fd7fc6e53203a5539717a8
---
 ReactCommon/fabric/uimanager/ShadowTree.cpp | 28 ++++++++++++---------
 ReactCommon/fabric/uimanager/ShadowTree.h   | 13 ++++++++--
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/ReactCommon/fabric/uimanager/ShadowTree.cpp b/ReactCommon/fabric/uimanager/ShadowTree.cpp
index 8717ea8fb..81c4a9ba2 100644
--- a/ReactCommon/fabric/uimanager/ShadowTree.cpp
+++ b/ReactCommon/fabric/uimanager/ShadowTree.cpp
@@ -62,27 +62,31 @@ void ShadowTree::synchronize(std::function<void(void)> function) const {
 #pragma mark - Layout
 
 Size ShadowTree::measure(const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const {
-  auto newRootShadowNode = cloneRootShadowNode(layoutConstraints, layoutContext);
+  auto newRootShadowNode = cloneRootShadowNode(getRootShadowNode(), layoutConstraints, layoutContext);
   newRootShadowNode->layout();
   return newRootShadowNode->getLayoutMetrics().frame.size;
 }
 
 bool ShadowTree::constraintLayout(const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const {
-  auto newRootShadowNode = cloneRootShadowNode(layoutConstraints, layoutContext);
-  return complete(newRootShadowNode);
+  auto oldRootShadowNode = getRootShadowNode();
+  auto newRootShadowNode = cloneRootShadowNode(oldRootShadowNode, layoutConstraints, layoutContext);
+  return complete(oldRootShadowNode, newRootShadowNode);
 }
 
 #pragma mark - Commiting
 
-UnsharedRootShadowNode ShadowTree::cloneRootShadowNode(const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const {
-  auto oldRootShadowNode = getRootShadowNode();
-  const auto &props = std::make_shared<const RootProps>(*oldRootShadowNode->getProps(), layoutConstraints, layoutContext);
+UnsharedRootShadowNode ShadowTree::cloneRootShadowNode(
+  const SharedRootShadowNode &oldRootShadowNode,
+  const LayoutConstraints &layoutConstraints,
+  const LayoutContext &layoutContext
+) const {
+  auto props = std::make_shared<const RootProps>(*oldRootShadowNode->getProps(), layoutConstraints, layoutContext);
   auto newRootShadowNode =
     std::make_shared<RootShadowNode>(*oldRootShadowNode, ShadowNodeFragment {.props = props});
   return newRootShadowNode;
 }
 
-bool  ShadowTree::complete(const SharedShadowNodeUnsharedList &rootChildNodes) const {
+bool ShadowTree::complete(const SharedShadowNodeUnsharedList &rootChildNodes) const {
   auto oldRootShadowNode = getRootShadowNode();
   auto newRootShadowNode =
     std::make_shared<RootShadowNode>(
@@ -92,14 +96,14 @@ bool  ShadowTree::complete(const SharedShadowNodeUnsharedList &rootChildNodes) c
       }
     );
 
-  return complete(newRootShadowNode);
+  return complete(oldRootShadowNode, newRootShadowNode);
 }
 
-bool ShadowTree::complete(UnsharedRootShadowNode newRootShadowNode) const {
-  SharedRootShadowNode oldRootShadowNode = getRootShadowNode();
-
+bool ShadowTree::complete(
+  const SharedRootShadowNode &oldRootShadowNode,
+  const UnsharedRootShadowNode &newRootShadowNode
+) const {
   newRootShadowNode->layout();
-
   newRootShadowNode->sealRecursive();
 
   auto mutations = calculateShadowViewMutations(
diff --git a/ReactCommon/fabric/uimanager/ShadowTree.h b/ReactCommon/fabric/uimanager/ShadowTree.h
index eedaabaee..d01a9c7af 100644
--- a/ReactCommon/fabric/uimanager/ShadowTree.h
+++ b/ReactCommon/fabric/uimanager/ShadowTree.h
@@ -87,14 +87,23 @@ public:
   ShadowTreeDelegate const *getDelegate() const;
 
 private:
+  UnsharedRootShadowNode cloneRootShadowNode(
+    const SharedRootShadowNode &oldRootShadowNode,
+    const LayoutConstraints &layoutConstraints,
+    const LayoutContext &layoutContext
+  ) const;
+
+  bool complete(
+    const SharedRootShadowNode &oldRootShadowNode,
+    const UnsharedRootShadowNode &newRootShadowNode
+  ) const;
 
-  UnsharedRootShadowNode cloneRootShadowNode(const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const;
-  bool complete(UnsharedRootShadowNode newRootShadowNode) const;
   bool commit(
     const SharedRootShadowNode &oldRootShadowNode,
     const SharedRootShadowNode &newRootShadowNode,
     const ShadowViewMutationList &mutations
   ) const;
+
   void toggleEventEmitters(const ShadowViewMutationList &mutations) const;
   void emitLayoutEvents(const ShadowViewMutationList &mutations) const;