Fabric: Safer UIManager deallocation and uninstallation

Summary: We have to uninstall UIManager synchronously to avoid a race condition when JS is capable to call already deallocated UIManager.

Reviewed By: mdvacca

Differential Revision: D10033406

fbshipit-source-id: 194d1ae2dd5ab09b036b1c165de289ada8e66014

Author: shergin

ReactCommon/fabric/events/EventBeat.cpp

@@ -19,12 +19,11 @@ void EventBeat::beat() const {
     return;
   }
 
-  if (!beatCallback_) {
-    return;
-  }
-
-  beatCallback_();
   isRequested_ = false;
+
+  if (beatCallback_) {
+    beatCallback_();
+  }
 }
 
 void EventBeat::induce() const {

ReactCommon/fabric/uimanager/FabricUIManager.cpp

@@ -104,9 +104,16 @@ static const std::string componentNameByReactViewName(std::string viewName) {
 }
 
 FabricUIManager::~FabricUIManager() {
-  (*executor_)([this] {
-    uninstaller_();
-  });
+  // We move `executor_` and `uninstaller_` inside a lambda to extend their
+  // life-time until the lambda finishes.
+  auto executor = std::shared_ptr<EventBeatBasedExecutor> {std::move(executor_)};
+  auto uninstaller = std::move(uninstaller_);
+
+  // We have to call this synchronously to postpose UIManager deallocation
+  // until it is fully uninstalled and JavaScript cannot access this anymore.
+  (*executor)([uninstaller, executor]() {
+    uninstaller();
+  }, EventBeatBasedExecutor::Mode::Synchronous);
 }
 
 void FabricUIManager::setComponentDescriptorRegistry(const SharedComponentDescriptorRegistry &componentDescriptorRegistry) {