From db4a7ae9b8245c60e6d820dc2d1b22d469867a51 Mon Sep 17 00:00:00 2001
From: Valentin Shergin <shergin@fb.com>
Date: Sun, 17 Nov 2019 13:52:41 -0800
Subject: [PATCH] Fabric: Attempt to recover from invariant violation in
 ~Sheduler

Summary:
We see crashes that happen in the destructor of `EventEmitterWrapper` class which indicates that `EventEmitter`s were not "disabled" before `Runtime` was deallocated. Assuming that `Scheduler` *was* deallocated before the `Runtime` (there is no proof for that), we can suppose that `stopSurface` was not called properly on time. Again, assuming that the moment of Scheduler deallocation is fine, we can try to recover from this situation by forced unmounting of all outstanding `ShadowTree` (that will make future destroying of EventEmitter don't crash).

Why do we believe that `Scheduler` *was* deallocated before `Runtime`?
We are not sure but that's possibly a case or that might be true for some portion of cases.
At least, we will know for sure what exactly went wrong. There is a possibility that all crashes that would happen after will happen exactly here.
And, frankly, from the probability standpoint, earlier we destroy JSI objects, lower chances are that it will be after the death of Runtime.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D18465408

fbshipit-source-id: 76338ed6fce0621cf11ce3178b4d1be7e0c73ccf
---
 ReactCommon/fabric/uimanager/Scheduler.cpp | 47 +++++++++++++++-------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/ReactCommon/fabric/uimanager/Scheduler.cpp b/ReactCommon/fabric/uimanager/Scheduler.cpp
index 9ebb0cee1d328f..f84e292df04c5b 100644
--- a/ReactCommon/fabric/uimanager/Scheduler.cpp
+++ b/ReactCommon/fabric/uimanager/Scheduler.cpp
@@ -85,27 +85,44 @@ Scheduler::Scheduler(
 }
 
 Scheduler::~Scheduler() {
-#ifndef NDEBUG
-  /*
-   * This requirement must be satisfied to make concurrent deallocation of
-   * `Scheduler` and calling on it from `UIManager` side thread-safe.
-   * Conceptually, `UIManager` is allowed to call `Scheduler` only if the
-   * corresponding `ShadowTree` instance exists (which is not fully enforced
-   * yet).
-   */
-  auto hasRunningSurfaces = false;
+  // All Surfaces must be explicitly stopped before destroying `Scheduler`.
+  // The idea is that `UIManager` is allowed to call `Scheduler` only if the
+  // corresponding `ShadowTree` instance exists.
+
+  // The thread-safety of this operation is guaranteed by this requirement.
+  uiManager_->setDelegate(nullptr);
+
+  // Then, let's verify that the requirement was satisfied.
+  auto surfaceIds = std::vector<SurfaceId>{};
   uiManager_->getShadowTreeRegistry().enumerate(
       [&](ShadowTree const &shadowTree, bool &stop) {
-        stop = true;
-        hasRunningSurfaces = true;
+        surfaceIds.push_back(shadowTree.getSurfaceId());
       });
 
   assert(
-      !hasRunningSurfaces &&
-      "Scheduler was destroyed with sill running Surfaces.");
-#endif
+      surfaceIds.size() == 0 &&
+      "Scheduler was destroyed with outstanding Surfaces.");
 
-  uiManager_->setDelegate(nullptr);
+  if (surfaceIds.size() == 0) {
+    return;
+  }
+
+  LOG(FATAL) << "Scheduler was destroyed with outstanding Surfaces.";
+
+  // If we are here, that means assert didn't fire which indicates that we in
+  // production.
+
+  // Now we have still-running surfaces, which is no good, no good.
+  // That's indeed a sign of a severe issue on the application layer.
+  // At this point, we don't have much to lose, so we are trying to unmount all
+  // outstanding `ShadowTree`s to prevent all stored JSI entities from
+  // overliving the `Scheduler`. (Unmounting `ShadowNode`s disables
+  // `EventEmitter`s which destroys JSI objects.)
+  for (auto surfaceId : surfaceIds) {
+    uiManager_->getShadowTreeRegistry().visit(
+        surfaceId,
+        [](ShadowTree const &shadowTree) { shadowTree.commitEmptyTree(); });
+  }
 }
 
 void Scheduler::startSurface(