Fabric: Lock-free events 2/n: Reimagining of EventTarget

Summary:
EventTargetWrapper and EventTarget were merged into one class that controls an `instanceHandle` reference and extracting a strong reference to it.

This diff also decouples the operation of retaining a strong reference (with checking a `enabled` flag) from actual usage of this reference. This allows to wrap into a mutex only first part of this process and avoid possible deadlocks.

Reviewed By: sahrens

Differential Revision: D13616382

fbshipit-source-id: 9907bc12047386fcf027929ae2ae41c0b727cd06

ReactCommon/fabric/events/EventEmitter.cpp

@@ -77,29 +77,26 @@ void EventEmitter::dispatchEvent(
       priority);
 }
 
-void EventEmitter::enable() const {
-  enableCounter_++;
-  toggleEventTargetOwnership_();
-}
+void EventEmitter::setEnabled(bool enabled) const {
+  enableCounter_ += enabled ? 1 : -1;
 
-void EventEmitter::disable() const {
-  enableCounter_--;
-  toggleEventTargetOwnership_();
-}
+  bool shouldBeEnabled = enableCounter_ > 0;
+  if (isEnabled_ != shouldBeEnabled) {
+    isEnabled_ = shouldBeEnabled;
+    if (eventTarget_) {
+      eventTarget_->setEnabled(isEnabled_);
+    }
+  }
 
-void EventEmitter::toggleEventTargetOwnership_() const {
   // Note: Initially, the state of `eventTarget_` and the value `enableCounter_`
   // is mismatched intentionally (it's `non-null` and `0` accordingly). We need
   // this to support an initial nebula state where the event target must be
   // retained without any associated mounted node.
   bool shouldBeRetained = enableCounter_ > 0;
-  bool alreadyBeRetained = eventTarget_ != nullptr;
-  if (shouldBeRetained == alreadyBeRetained) {
-    return;
-  }
-
-  if (!shouldBeRetained) {
-    eventTarget_.reset();
+  if (shouldBeRetained != (eventTarget_ != nullptr)) {
+    if (!shouldBeRetained) {
+      eventTarget_.reset();
+    }
   }
 }
 

ReactCommon/fabric/events/EventEmitter.h

@@ -25,15 +25,6 @@ using SharedEventEmitter = std::shared_ptr<const EventEmitter>;
  * Stores a pointer to `EventTarget` identifying a particular component and
  * a weak pointer to `EventDispatcher` which is responsible for delivering the
  * event.
- *
- * Note: Retaining an `EventTarget` does *not* guarantee that actual event
- * target exists and/or valid in JavaScript realm. The `EventTarget` retains an
- * `EventTargetWrapper` which wraps JavaScript object in `unsafe-unretained`
- * manner. Retaining the `EventTarget` *does* indicate that we can use that to
- * get an actual JavaScript object from that in the future *ensuring safety
- * beforehand somehow*; JSI maintains `WeakObject` object as long as we retain
- * the `EventTarget`. All `EventTarget` instances must be deallocated before
- * stopping JavaScript machine.
  */
 class EventEmitter {
   /*
@@ -58,12 +49,13 @@ class EventEmitter {
    * `DispatchMutex` must be acquired before calling.
    * Enables/disables event emitter.
    * Enabled event emitter retains a pointer to `eventTarget` strongly (as
-   * `std::shared_ptr`) whereas disabled one weakly (as `std::weak_ptr`).
+   * `std::shared_ptr`) whereas disabled one don't.
+   * Enabled/disabled state is also proxied to `eventTarget` where it indicates
+   * a possibility to extract JSI value from it.
    * The enable state is additive; a number of `enable` calls should be equal to
    * a number of `disable` calls to release the event target.
    */
-  void enable() const;
-  void disable() const;
+  void setEnabled(bool enabled) const;
 
  protected:
 #ifdef ANDROID
@@ -93,6 +85,7 @@ class EventEmitter {
   mutable SharedEventTarget eventTarget_;
   WeakEventDispatcher eventDispatcher_;
   mutable int enableCounter_{0};
+  mutable bool isEnabled_{false};
 };
 
 } // namespace react

ReactCommon/fabric/events/EventQueue.cpp

@@ -43,13 +43,16 @@ void EventQueue::onBeat(jsi::Runtime &runtime) const {
     std::lock_guard<std::recursive_mutex> lock(EventEmitter::DispatchMutex());
 
     for (const auto &event : queue) {
-      eventPipe_(
-          runtime,
-          event.eventTarget.lock().get(),
-          event.type,
-          event.payloadFactory);
+      if (event.eventTarget) {
+        event.eventTarget->retain(runtime);
+      }
     }
   }
+
+  for (const auto &event : queue) {
+    eventPipe_(
+        runtime, event.eventTarget.get(), event.type, event.payloadFactory);
+  }
 }
 
 } // namespace react

ReactCommon/fabric/events/EventTarget.cpp

@@ -0,0 +1,48 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#include "EventTarget.h"
+
+namespace facebook {
+namespace react {
+
+using Tag = EventTarget::Tag;
+
+EventTarget::EventTarget(
+    jsi::Runtime &runtime,
+    const jsi::Value &instanceHandle,
+    Tag tag)
+    : weakInstanceHandle_(
+          jsi::WeakObject(runtime, instanceHandle.asObject(runtime))),
+      strongInstanceHandle_(jsi::Value::null()),
+      tag_(tag) {}
+
+void EventTarget::setEnabled(bool enabled) const {
+  enabled_ = enabled;
+}
+
+void EventTarget::retain(jsi::Runtime &runtime) const {
+  if (!enabled_) {
+    return;
+  }
+
+  strongInstanceHandle_ = weakInstanceHandle_.lock(runtime);
+}
+
+jsi::Value EventTarget::release(jsi::Runtime &runtime) const {
+  // The method does not use `jsi::Runtime` reference.
+  // It takes it only to ensure thread-safety (if the caller has the reference,
+  // we are on a proper thread).
+  return std::move(strongInstanceHandle_);
+}
+
+Tag EventTarget::getTag() const {
+  return tag_;
+}
+
+} // namespace react
+} // namespace facebook

ReactCommon/fabric/events/EventTarget.h

@@ -0,0 +1,72 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+#pragma once
+
+#include <memory>
+
+#include <jsi/jsi.h>
+
+namespace facebook {
+namespace react {
+
+/*
+ * `EventTarget` represents storage of a weak instance handle object with some
+ * information about the possibility of retaining that strongly on demand.
+ * Note: Retaining an `EventTarget` does *not* guarantee that actual event
+ * target (a JavaScript object) is retaining and/or valid in JavaScript realm.
+ * The `EventTarget` retains an `instanceHandle` value in `unsafe-unretained`
+ * manner.
+ * All `EventTarget` instances must be deallocated before
+ * stopping JavaScript machine.
+ */
+class EventTarget {
+ public:
+  /*
+   * We have to repeat `Tag` type definition here because `events` module does
+   * not depend on `core` module (and should not).
+   */
+  using Tag = int32_t;
+
+  /*
+   * Constructs an EventTarget from a weak instance handler and a tag.
+   */
+  EventTarget(jsi::Runtime &runtime, const jsi::Value &instanceHandle, Tag tag);
+
+  /*
+   * Sets the `enabled` flag that allows creating a strong instance handle from
+   * a weak one.
+   */
+  void setEnabled(bool enabled) const;
+
+  /*
+   * Creates a strong instance handle from a weak one and stores it inside the
+   * object.
+   * If the EventTarget is disabled, does nothing.
+   */
+  void retain(jsi::Runtime &runtime) const;
+
+  /*
+   * Extract the stored strong instance handle from the object and returns it.
+   */
+  jsi::Value release(jsi::Runtime &runtime) const;
+
+  /*
+   * Deprecated. Do not use.
+   */
+  Tag getTag() const;
+
+ private:
+  mutable bool enabled_{false}; // Protected by `EventEmitter::DispatchMutex()`.
+  mutable jsi::WeakObject weakInstanceHandle_; // Protected by `jsi::Runtime &`.
+  mutable jsi::Value strongInstanceHandle_; // Protected by `jsi::Runtime &`.
+  Tag tag_;
+};
+
+using SharedEventTarget = std::shared_ptr<const EventTarget>;
+
+} // namespace react
+} // namespace facebook

ReactCommon/fabric/events/RawEvent.cpp

@@ -13,7 +13,7 @@ namespace react {
 RawEvent::RawEvent(
     std::string type,
     ValueFactory payloadFactory,
-    WeakEventTarget eventTarget)
+    SharedEventTarget eventTarget)
     : type(std::move(type)),
       payloadFactory(std::move(payloadFactory)),
       eventTarget(std::move(eventTarget)) {}

ReactCommon/fabric/events/RawEvent.h

@@ -23,11 +23,11 @@ class RawEvent {
   RawEvent(
       std::string type,
       ValueFactory payloadFactory,
-      WeakEventTarget eventTarget);
+      SharedEventTarget eventTarget);
 
   const std::string type;
   const ValueFactory payloadFactory;
-  const WeakEventTarget eventTarget;
+  const SharedEventTarget eventTarget;
 };
 
 } // namespace react

ReactCommon/fabric/events/primitives.h

@@ -10,6 +10,8 @@
 #include <folly/dynamic.h>
 #include <jsi/jsi.h>
 
+#include <react/events/EventTarget.h>
+
 namespace facebook {
 namespace react {
 
@@ -33,20 +35,12 @@ enum class EventPriority : int {
  * `EventHandler` is managed as a `unique_ptr`, so it must have a *virtual*
  * destructor to allow proper deallocation having only a pointer
  * to the base (`EventHandler`) class.
- *
- * `EventTarget` is managed as a `shared_ptr`, so it does not need to have a
- * virtual destructor because `shared_ptr` stores a pointer to destructor
- * inside.
  */
 struct EventHandler {
   virtual ~EventHandler() = default;
 };
 using UniqueEventHandler = std::unique_ptr<const EventHandler>;
 
-struct EventTarget {};
-using SharedEventTarget = std::shared_ptr<const EventTarget>;
-using WeakEventTarget = std::weak_ptr<const EventTarget>;
-
 using ValueFactory = std::function<jsi::Value(jsi::Runtime &runtime)>;
 
 using EventPipe = std::function<void(

ReactCommon/fabric/uimanager/ShadowTree.cpp

@@ -159,13 +159,13 @@ void ShadowTree::toggleEventEmitters(
 
   for (const auto &mutation : mutations) {
     if (mutation.type == ShadowViewMutation::Create) {
-      mutation.newChildShadowView.eventEmitter->enable();
+      mutation.newChildShadowView.eventEmitter->setEnabled(true);
     }
   }
 
   for (const auto &mutation : mutations) {
     if (mutation.type == ShadowViewMutation::Delete) {
-      mutation.oldChildShadowView.eventEmitter->disable();
+      mutation.oldChildShadowView.eventEmitter->setEnabled(false);
     }
   }
 }

ReactCommon/fabric/uimanager/UIManagerBinding.cpp

@@ -71,27 +71,27 @@ void UIManagerBinding::dispatchEvent(
   SystraceSection s("UIManagerBinding::dispatchEvent");
 
   auto payload = payloadFactory(runtime);
-  auto eventTargetValue = jsi::Value::null();
 
-  if (eventTarget) {
-    auto &eventTargetWrapper =
-        static_cast<const EventTargetWrapper &>(*eventTarget);
-    eventTargetValue = eventTargetWrapper.instanceHandle.lock(runtime);
-    if (eventTargetValue.isUndefined()) {
-      return;
-    }
+  auto instanceHandle = eventTarget
+    ? [&]() {
+      auto instanceHandle = eventTarget->release(runtime);
+      if (instanceHandle.isUndefined()) {
+        return jsi::Value::null();
+      }
 
-    // Mixing `target` into `payload`.
-    assert(payload.isObject());
-    payload.asObject(runtime).setProperty(
-        runtime, "target", eventTargetWrapper.tag);
-  }
+      // Mixing `target` into `payload`.
+      assert(payload.isObject());
+      payload.asObject(runtime).setProperty(runtime, "target", eventTarget->getTag());
+      return instanceHandle;
+    }()
+    : jsi::Value::null();
 
   auto &eventHandlerWrapper =
       static_cast<const EventHandlerWrapper &>(*eventHandler_);
+
   eventHandlerWrapper.callback.call(
       runtime,
-      {std::move(eventTargetValue),
+      {std::move(instanceHandle),
        jsi::String::createFromUtf8(runtime, type),
        std::move(payload)});
 }

ReactCommon/fabric/uimanager/primitives.h

@@ -30,14 +30,6 @@ inline RawProps rawPropsFromDynamic(const folly::dynamic object) noexcept {
   return result;
 }
 
-struct EventTargetWrapper : public EventTarget {
-  EventTargetWrapper(jsi::WeakObject instanceHandle, Tag tag)
-      : instanceHandle(std::move(instanceHandle)), tag(tag) {}
-
-  mutable jsi::WeakObject instanceHandle;
-  mutable Tag tag;
-};
-
 struct EventHandlerWrapper : public EventHandler {
   EventHandlerWrapper(jsi::Function eventHandler)
       : callback(std::move(eventHandler)) {}
@@ -100,9 +92,8 @@ inline static SharedEventTarget eventTargetFromValue(
     jsi::Runtime &runtime,
     const jsi::Value &eventTargetValue,
     const jsi::Value &tagValue) {
-  return std::make_shared<EventTargetWrapper>(
-      jsi::WeakObject(runtime, eventTargetValue.getObject(runtime)),
-      tagValue.getNumber());
+  return std::make_shared<EventTarget>(
+      runtime, eventTargetValue, tagValue.getNumber());
 }
 
 inline static Tag tagFromValue(jsi::Runtime &runtime, const jsi::Value &value) {