From 28a5f122a8cfc9bd38eaa88e0932d9bd94822e03 Mon Sep 17 00:00:00 2001
From: Valentin Shergin <shergin@fb.com>
Date: Mon, 9 Sep 2019 20:22:55 -0700
Subject: [PATCH] Fabric: `MountingCoordinator::revoke()`

Summary:
MountingCoordinator is a borderline between Core and Mounting. Some of Core design constraints are impossible/impractical to enforce on Mounting layer, so we have to handle all of those cases in `MountingCoordinator`.

One of the constrains is that all ShadowNodes implicitly depend on associated ComponentDescriptor instances without retaining them (retaining is expensive and creates a retain cycle).
The problem is that the Mounting layer can call `MountingCoordinator::pull()` at any moment (even after the whole Core is already destroyed). To prevent this, the owner of a `MountingCoordinator` on the Core side calls `revoke()` right before being deallocated (right before the moment the owner cannot guarantee the constraint).

Reviewed By: JoshuaGross

Differential Revision: D17272295

fbshipit-source-id: ba8b02eab8f84cce68aa65c1ad36950cd2498049
---
 ReactCommon/fabric/core/layout/LayoutableShadowNode.h | 2 +-
 ReactCommon/fabric/mounting/MountingCoordinator.cpp   | 5 +++++
 ReactCommon/fabric/mounting/MountingCoordinator.h     | 9 +++++++++
 ReactCommon/fabric/mounting/ShadowTree.cpp            | 1 +
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/ReactCommon/fabric/core/layout/LayoutableShadowNode.h b/ReactCommon/fabric/core/layout/LayoutableShadowNode.h
index 427f443e21d..3aec94a6f42 100644
--- a/ReactCommon/fabric/core/layout/LayoutableShadowNode.h
+++ b/ReactCommon/fabric/core/layout/LayoutableShadowNode.h
@@ -83,7 +83,7 @@ class LayoutableShadowNode : public virtual Sealable {
   /*
    * Clean or Dirty layout state:
    * Indicates whether all nodes (and possibly their subtrees) along the path
-   * to the root node should be re-layouted.
+   * to the root node should be re-laid out.
    */
   virtual void cleanLayout() = 0;
   virtual void dirtyLayout() = 0;
diff --git a/ReactCommon/fabric/mounting/MountingCoordinator.cpp b/ReactCommon/fabric/mounting/MountingCoordinator.cpp
index 47ecf719f55..9cb77d049ce 100644
--- a/ReactCommon/fabric/mounting/MountingCoordinator.cpp
+++ b/ReactCommon/fabric/mounting/MountingCoordinator.cpp
@@ -44,6 +44,11 @@ void MountingCoordinator::push(ShadowTreeRevision &&revision) const {
   }
 }
 
+void MountingCoordinator::revoke() const {
+  std::lock_guard<std::mutex> lock(mutex_);
+  lastRevision_.reset();
+}
+
 better::optional<MountingTransaction> MountingCoordinator::pullTransaction()
     const {
   std::lock_guard<std::mutex> lock(mutex_);
diff --git a/ReactCommon/fabric/mounting/MountingCoordinator.h b/ReactCommon/fabric/mounting/MountingCoordinator.h
index 4fe2bddf310..9bbdda13532 100644
--- a/ReactCommon/fabric/mounting/MountingCoordinator.h
+++ b/ReactCommon/fabric/mounting/MountingCoordinator.h
@@ -60,6 +60,15 @@ class MountingCoordinator final {
    */
   void push(ShadowTreeRevision &&revision) const;
 
+  /*
+   * Revokes the last pushed `ShadowTreeRevision`.
+   * Generating a `MountingTransaction` requires some resources which the
+   * `MountingCoordinator` does not own (e.g. `ComponentDescriptor`s). Revoking
+   * committed revisions allows the owner (a Shadow Tree) to make sure that
+   * those resources will not be accessed (e.g. by the Mouting Layer).
+   */
+  void revoke() const;
+
  private:
   SurfaceId const surfaceId_;
 
diff --git a/ReactCommon/fabric/mounting/ShadowTree.cpp b/ReactCommon/fabric/mounting/ShadowTree.cpp
index af08afd792b..57221095f62 100644
--- a/ReactCommon/fabric/mounting/ShadowTree.cpp
+++ b/ReactCommon/fabric/mounting/ShadowTree.cpp
@@ -116,6 +116,7 @@ ShadowTree::~ShadowTree() {
                 /* .children = */ ShadowNode::emptySharedShadowNodeSharedList(),
             });
       });
+  mountingCoordinator_->revoke();
 }
 
 Tag ShadowTree::getSurfaceId() const {