Fabric: Fixed incorrect memory management in Scheduler

Summary:
The previous implementation was incorrect causing a memory leak. In the new approach, we use a shared pointer to optional to be able to construct that ahead of time and then fill with actual value.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D21516794

fbshipit-source-id: ddcbf8a1ad2b1f629ae4ff2842edeb7bb2a275a6

ReactCommon/fabric/scheduler/Scheduler.cpp

@@ -29,13 +29,13 @@ Scheduler::Scheduler(
       schedulerToolbox.contextContainer
           ->at<std::shared_ptr<const ReactNativeConfig>>("ReactNativeConfig");
 
+  // Creating a container for future `EventDispatcher` instance.
+  eventDispatcher_ =
+      std::make_shared<better::optional<EventDispatcher const>>();
+
   auto uiManager = std::make_shared<UIManager>();
   auto eventOwnerBox = std::make_shared<EventBeat::OwnerBox>();
-
-  // A dummy pointer to share a control block (and life-time) with
-  // an actual `owner` later.
-  auto owner = std::make_shared<bool const>(false);
-  eventOwnerBox->owner = owner;
+  eventOwnerBox->owner = eventDispatcher_;
 
   auto eventPipe = [uiManager](
                        jsi::Runtime &runtime,
@@ -52,21 +52,24 @@ Scheduler::Scheduler(
     uiManager->updateState(stateUpdate);
   };
 
-  auto eventDispatcher = std::make_unique<EventDispatcher const>(
+  // Creating an `EventDispatcher` instance inside the already allocated
+  // container (inside the optional).
+  eventDispatcher_->emplace(
       eventPipe,
       statePipe,
       schedulerToolbox.synchronousEventBeatFactory,
       schedulerToolbox.asynchronousEventBeatFactory,
       eventOwnerBox);
 
-  eventDispatcher_ =
-      std::shared_ptr<EventDispatcher const>(owner, eventDispatcher.release());
+  // Casting to `std::shared_ptr<EventDispatcher const>`.
+  auto eventDispatcher =
+      EventDispatcher::Shared{eventDispatcher_, &eventDispatcher_->value()};
 
   componentDescriptorRegistry_ = schedulerToolbox.componentRegistryFactory(
-      eventDispatcher_, schedulerToolbox.contextContainer);
+      eventDispatcher, schedulerToolbox.contextContainer);
 
   rootComponentDescriptor_ = std::make_unique<const RootComponentDescriptor>(
-      ComponentDescriptorParameters{eventDispatcher_, nullptr, nullptr});
+      ComponentDescriptorParameters{eventDispatcher, nullptr, nullptr});
 
   uiManager->setDelegate(this);
   uiManager->setComponentDescriptorRegistry(componentDescriptorRegistry_);

ReactCommon/fabric/scheduler/Scheduler.h

@@ -111,7 +111,16 @@ class Scheduler final : public UIManagerDelegate {
   RuntimeExecutor runtimeExecutor_;
   std::shared_ptr<UIManager> uiManager_;
   std::shared_ptr<const ReactNativeConfig> reactNativeConfig_;
-  EventDispatcher::Shared eventDispatcher_;
+
+  /*
+   * At some point, we have to have an owning shared pointer to something that
+   * will become an `EventDispatcher` a moment later. That's why we have it as a
+   * pointer to an optional: we construct the pointer first, share that with
+   * parts that need to have ownership (and only ownership) of that, and then
+   * fill the optional.
+   */
+  std::shared_ptr<better::optional<EventDispatcher const>> eventDispatcher_;
+
   bool enableNewStateReconciliation_{false};
 };