EventQueueProcessor/EventTarget: SCOPE_EXIT-guard release on throwing dispatch + fix unsigned retainCount_ underflow (#56537)

Summary:
**1) `EventQueueProcessor::flushEvents` — release on every exit path.** `flushEvents` retains every `EventTarget` in the batch, then for each event calls `eventPipe_` (which dispatches into JS via `UIManagerBinding::dispatchEventToJS` and can throw `jsi::JSError`), then `eventPipeConclusion_`, and finally runs a trailing loop that calls `event.eventTarget->release(runtime)`. If either pipe call throws, stack unwinding skips the release loop, leaving `strongInstanceHandle_` (a `jsi::Value` strong reference to the JS event-target object) pinned and `retainCount_` desynchronised.

This change replaces the trailing release loop with a folly `SCOPE_EXIT` block immediately after the retain pass, so every retained target is released on every exit path — normal return, `eventPipe_` throw, or `eventPipeConclusion_` throw. (Earlier revisions used a hand-rolled RAII class; switched to `SCOPE_EXIT` per reviewer feedback. `folly/ScopeGuard.h` is already in the OSS RN folly subset — `RCT-Folly.podspec`, `ReactAndroid/.../folly/CMakeLists.txt`, `react-native-fantom/.../folly/CMakeLists.txt` — and is a transitive dep of `//xplat/folly:dynamic`, so no new build edges.)

Behaviour-preserving on the happy path: same release ordering (after `eventPipeConclusion_`), same no-`DispatchMutex` policy for release (the `events` vector still holds the strong `shared_ptr`s), zero added allocation.

**2) `EventTarget::release` — guard the unsigned decrement.** `retainCount_` is `size_t`. `retain()` early-returns when the target is disabled, but the matching `release()` always runs, so `release()` can be entered with `retainCount_ == 0`; the unconditional `--retainCount_` then wraps to `SIZE_MAX`, the `== 0` check never fires again, and `strongInstanceHandle_` is never cleared. The trailing `react_native_assert(retainCount_ >= 0)` is a tautology on an unsigned type and is removed. This change guards the decrement (`if (retainCount_ > 0) --retainCount_;`) and drops the dead assert and the now-unused `react_native_assert.h` include.

Adds `EventQueueProcessorTest.releasesEventTargetsWhenDispatchThrows` (proves the strong handle is released during unwind when `eventPipe_` throws; fails on the pre-fix trailing-loop code) and `EventTargetTests.releaseWhileDisabledDoesNotCorruptRetainCount` (retain/release while disabled, then enable+retain must still acquire the handle; fails on the pre-fix unconditional decrement).

Changelog: [Internal]

Pull Request resolved: #56537

Reviewed By: javache

Differential Revision: D100280132

fbshipit-source-id: 8c7128082d8c967a150952434a7791b62586659f

Author: meta-yaohway

packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp

@@ -7,6 +7,7 @@
 
 #include "EventQueueProcessor.h"
 
+#include <folly/ScopeGuard.h>
 #include <logger/react_native_log.h>
 #include <react/featureflags/ReactNativeFeatureFlags.h>
 
@@ -38,6 +39,20 @@ void EventQueueProcessor::flushEvents(
     }
   }
 
+  // RAII guard for the matching release() pass. If event dispatch throws (a JS
+  // exception from eventPipe_, or eventPipeConclusion_), the previous code
+  // skipped the release loop entirely, leaking JSI strong references. The
+  // guard destructor runs on every exit path. No DispatchMutex needed for
+  // release: we hold a strong pointer to each EventTarget via `events`, and
+  // release() only touches runtime-thread-confined state.
+  SCOPE_EXIT {
+    for (const auto& event : events) {
+      if (event.eventTarget) {
+        event.eventTarget->release(runtime);
+      }
+    }
+  };
+
   for (const auto& event : events) {
     auto reactPriority = ReactEventPriority::Default;
 
@@ -111,15 +126,7 @@ void EventQueueProcessor::flushEvents(
   // We only run the "Conclusion" once per event group when batched.
   eventPipeConclusion_(runtime);
 
-  // No need to lock `EventEmitter::DispatchMutex()` here.
-  // The mutex protects from a situation when the `instanceHandle` can be
-  // deallocated during accessing, but that's impossible at this point because
-  // we have a strong pointer to it.
-  for (const auto& event : events) {
-    if (event.eventTarget) {
-      event.eventTarget->release(runtime);
-    }
-  }
+  // EventTarget release happens in the SCOPE_EXIT above.
 }
 
 void EventQueueProcessor::flushStateUpdates(

packages/react-native/ReactCommon/react/renderer/core/EventTarget.cpp

@@ -7,8 +7,6 @@
 
 #include "EventTarget.h"
 
-#include <react/debug/react_native_assert.h>
-
 namespace facebook::react {
 
 using Tag = EventTarget::Tag;
@@ -51,11 +49,16 @@ void EventTarget::release(jsi::Runtime& /*runtime*/) {
   // It takes it only to ensure thread-safety (if the caller has the reference,
   // we are on a proper thread).
 
-  if (--retainCount_ == 0) {
+  // retainCount_ is unsigned. retain() early-returns when the target is
+  // disabled but the matching release() still runs, so we can land here with
+  // a zero count; an unconditional decrement would wrap to SIZE_MAX and the
+  // strong handle would never be cleared again.
+  if (retainCount_ > 0) {
+    --retainCount_;
+  }
+  if (retainCount_ == 0) {
     strongInstanceHandle_ = jsi::Value::null();
   }
-
-  react_native_assert(retainCount_ >= 0);
 }
 
 jsi::Value EventTarget::getInstanceHandle(jsi::Runtime& runtime) const {

packages/react-native/ReactCommon/react/renderer/core/tests/EventQueueProcessorTest.cpp

@@ -12,11 +12,13 @@
 #include <react/renderer/core/EventPipe.h>
 #include <react/renderer/core/EventQueueProcessor.h>
 #include <react/renderer/core/EventTarget.h>
+#include <react/renderer/core/InstanceHandle.h>
 #include <react/renderer/core/ShadowNodeFamily.h>
 #include <react/renderer/core/StatePipe.h>
 #include <react/renderer/core/ValueFactoryEventPayload.h>
 
 #include <memory>
+#include <stdexcept>
 #include <string_view>
 
 namespace facebook::react {
@@ -157,4 +159,50 @@ TEST_F(EventQueueProcessorTest, alwaysDiscreteEvent) {
   EXPECT_EQ(eventPriorities_[0], ReactEventPriority::Discrete);
 }
 
+TEST_F(EventQueueProcessorTest, releasesEventTargetsWhenDispatchThrows) {
+  // Set up an enabled EventTarget so that flushEvents() will retain a strong
+  // JSI reference to its instance handle.
+  auto object = jsi::Object(*runtime_);
+  auto instanceHandle = std::make_shared<InstanceHandle>(
+      *runtime_, jsi::Value(*runtime_, object), 1);
+  auto eventTarget =
+      std::make_shared<EventTarget>(std::move(instanceHandle), 41);
+  eventTarget->setEnabled(true);
+
+  // An event pipe that throws, simulating a JS exception during dispatch.
+  auto throwingEventPipe = [](jsi::Runtime& /*runtime*/,
+                              const EventTarget* /*eventTarget*/,
+                              const std::string& /*type*/,
+                              ReactEventPriority /*priority*/,
+                              const EventPayload& /*payload*/,
+                              HighResTimeStamp /*eventTimestamp*/) {
+    throw std::runtime_error("dispatch failed");
+  };
+  auto dummyEventPipeConclusion = [](jsi::Runtime& /*runtime*/) {};
+  auto dummyStatePipe = [](const StateUpdate& /*stateUpdate*/) {};
+  auto mockEventLogger = std::make_shared<MockEventLogger>();
+
+  auto processor = EventQueueProcessor(
+      throwingEventPipe,
+      dummyEventPipeConclusion,
+      dummyStatePipe,
+      mockEventLogger);
+
+  EXPECT_THROW(
+      processor.flushEvents(
+          *runtime_,
+          {RawEvent(
+              "onThrow",
+              std::make_shared<ValueFactoryEventPayload>(dummyValueFactory_),
+              eventTarget,
+              {},
+              RawEvent::Category::Discrete)}),
+      std::runtime_error);
+
+  // The strong JSI reference acquired by retain() must have been released even
+  // though dispatch threw, so the instance handle is no longer reachable
+  // through the EventTarget.
+  EXPECT_TRUE(eventTarget->getInstanceHandle(*runtime_).isNull());
+}
+
 } // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/core/tests/EventTargetTests.cpp

@@ -43,3 +43,29 @@ TEST(EventTargetTests, getInstanceHandle) {
 
   EXPECT_TRUE(eventTarget.getInstanceHandle(*runtime).isNull());
 }
+
+TEST(EventTargetTests, releaseWhileDisabledDoesNotCorruptRetainCount) {
+  // retain() is a no-op while the target is disabled, but the matching
+  // release() still runs. release() must tolerate a zero retain count without
+  // underflowing, so that a later enable + retain still populates the strong
+  // instance handle.
+  auto runtime = facebook::hermes::makeHermesRuntime();
+  auto object = jsi::Object(*runtime);
+  auto instanceHandle = std::make_shared<InstanceHandle>(
+      *runtime, jsi::Value(*runtime, object), 1);
+
+  auto eventTarget = EventTarget(std::move(instanceHandle), 41);
+
+  // Disabled by default: retain is a no-op, release sees a zero count.
+  eventTarget.retain(*runtime);
+  eventTarget.release(*runtime);
+
+  // After enabling, a fresh retain must succeed in acquiring the handle.
+  eventTarget.setEnabled(true);
+  eventTarget.retain(*runtime);
+  EXPECT_FALSE(eventTarget.getInstanceHandle(*runtime).isNull());
+
+  // And the matching release must drop it again.
+  eventTarget.release(*runtime);
+  EXPECT_TRUE(eventTarget.getInstanceHandle(*runtime).isNull());
+}