Make codegen'd TurboModule event emitters no-op when the emitter callback is absent (#57893)

Summary: Pull Request resolved: https://github.com/react/react-native/pull/57893 The codegen'd TurboModule `emitOn<Event>` methods invoked their `EventEmitterCallback` without checking it was set. That callback is installed by the generated `*SpecJSI` constructor, which runs when JS first looks the module up — so a native module that emits before that point invoked an empty `std::function` on iOS (`std::bad_function_call`) or a null field on Android (NPE). Native code commonly holds the module instance and pushes events well before any JS surface mounts, so call sites had to wrap every emit in a try/catch to stay crash-free. Both generators now read the callback into a local and no-op when it is absent: - ObjC++ (`serializeEventEmitter.js`): copies `_eventEmitterCallback`, calls it only if non-empty. - Java (`GenerateModuleJavaSpec.js`): copies the `Nullable CxxCallbackImpl` field and null-checks it. The local is for readability, not synchronization: the callback is installed once and never cleared, and Java reference reads are already atomic. The `setEventEmitterCallback` lambdas had a separate lifetime bug: they captured `eventEmitterMap_` by reference and looked events up with `operator[]`. The Java/ObjC module owns the callback and can outlive the C++ `*SpecJSI` that installed it, so a stale callback dereferenced a dangling map; and `operator[]` silently default-inserted a null `shared_ptr` for an unknown event name, which the next line dereferenced. They now capture a copy of the map and use a checked `find` (`serializeModule.js`, `JavaTurboModule.cpp`). Every emitter is registered before the callback is installed, so the copy is complete. Note the scope of the guarantee: it covers the ObjC++ and Java generators, which route through an `EventEmitterCallback`. A C++-only TurboModule (`<Module>CxxSpec`, from `GenerateModuleH.js`) has no callback to check — it emits through `eventEmitterMap_` entries its own constructor registers — so the "emit unconditionally" guidance is about the callback, not about emitting before construction finishes. Changelog: [General][Fixed] - TurboModule event emitters no longer throw when an event is emitted before the emitter callback is installed Reviewed By: javache Differential Revision: D115130767 fbshipit-source-id: 7d7b9a706b8983968e3e1207b22c66322f04a223
2026-09-28 13:23:09 +08:00 · 2026-08-17 10:41:02 -07:00
parent c467843ed0
commit 4bf5575490
7 changed files with 174 additions and 39 deletions
@@ -82,16 +82,25 @@ function EventEmitterTemplate(
  eventEmitter: NativeModuleEventEmitterShape,
  imports: Set<string>,
 ): string {
+  imports.add('com.facebook.react.bridge.CxxCallbackImpl');
+  // mEventEmitterCallback is set from JNI by configureEventEmitterCallback(),
+  // which the generated SpecJSI constructor calls when JS first looks the module
+  // up. Emitting before that would hit a null field, so the emitted code no-ops
+  // instead. The local is for readability: reference reads are already atomic,
+  // and the field is never reset to null.
  return `  protected final void emit${toPascalCase(eventEmitter.name)}(${
    eventEmitter.typeAnnotation.typeAnnotation.type !== 'VoidTypeAnnotation'
      ? `${translateEventEmitterTypeToJavaType(eventEmitter, imports)} value`
      : ''
  }) {
-    mEventEmitterCallback.invoke("${eventEmitter.name}"${
-      eventEmitter.typeAnnotation.typeAnnotation.type !== 'VoidTypeAnnotation'
-        ? ', value'
-        : ''
-    });
+    CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
+    if (eventEmitterCallback != null) {
+      eventEmitterCallback.invoke("${eventEmitter.name}"${
+        eventEmitter.typeAnnotation.typeAnnotation.type !== 'VoidTypeAnnotation'
+          ? ', value'
+          : ''
+      });
+    }
  }`;
 }

@@ -78,20 +78,28 @@ function EventEmitterHeaderTemplate(
 function EventEmitterImplementationTemplate(
  eventEmitter: NativeModuleEventEmitterShape,
 ): string {
+  // _eventEmitterCallback is installed by the generated SpecJSI constructor,
+  // which runs when JS first looks the module up. Emitting before that would
+  // call an empty std::function, so the emitted code no-ops instead. The local
+  // copy is for readability; it does not synchronize against a concurrent
+  // install.
  return `- (void)emit${toPascalCase(eventEmitter.name)}${
    eventEmitter.typeAnnotation.typeAnnotation.type !== 'VoidTypeAnnotation'
      ? `:(${getEventEmitterTypeObjCType(eventEmitter)})value`
      : ''
  }
 {
-  _eventEmitterCallback("${eventEmitter.name}", ${
-    eventEmitter.typeAnnotation.typeAnnotation.type !== 'VoidTypeAnnotation'
-      ? eventEmitter.typeAnnotation.typeAnnotation.type !==
-        'BooleanTypeAnnotation'
-        ? 'value'
-        : '[NSNumber numberWithBool:value]'
-      : 'nil'
-  });
+  auto eventEmitterCallback = _eventEmitterCallback;
+  if (eventEmitterCallback) {
+    eventEmitterCallback("${eventEmitter.name}", ${
+      eventEmitter.typeAnnotation.typeAnnotation.type !== 'VoidTypeAnnotation'
+        ? eventEmitter.typeAnnotation.typeAnnotation.type !==
+          'BooleanTypeAnnotation'
+          ? 'value'
+          : '[NSNumber numberWithBool:value]'
+        : 'nil'
+    });
+  }
 }`;
 }

@@ -83,10 +83,17 @@ namespace facebook::react {
              .join('')
          : ''
      }${
+        // The callback outlives this module, so it captures a copy of the map
+        // instead of referencing eventEmitterMap_. Every emitter is registered
+        // directly above, so the copy is complete; an emitter registered after
+        // construction would not be reachable from the callback.
        eventEmitters.length > 0
          ? `
-        setEventEmitterCallback([&](const std::string &name, id value) {
-          static_cast<AsyncEventEmitter<id> &>(*eventEmitterMap_[name]).emit(value);
+        setEventEmitterCallback([eventEmitterMap = eventEmitterMap_](const std::string &name, id value) {
+          auto it = eventEmitterMap.find(name);
+          if (it != eventEmitterMap.end() && it->second) {
+            static_cast<AsyncEventEmitter<id> &>(*it->second).emit(value);
+          }
        });`
          : ''
      }
@@ -229,6 +229,7 @@ Map {
 package com.facebook.fbreact.specs;

 import com.facebook.proguard.annotations.DoNotStrip;
+import com.facebook.react.bridge.CxxCallbackImpl;
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.ReactContextBaseJavaModule;
 import com.facebook.react.bridge.ReactMethod;
@@ -250,27 +251,45 @@ public abstract class NativeSampleTurboModuleSpec extends ReactContextBaseJavaMo
  }

  protected final void emitOnEvent1() {
-    mEventEmitterCallback.invoke(\\"onEvent1\\");
+    CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
+    if (eventEmitterCallback != null) {
+      eventEmitterCallback.invoke(\\"onEvent1\\");
+    }
  }

  protected final void emitOnEvent2(String value) {
-    mEventEmitterCallback.invoke(\\"onEvent2\\", value);
+    CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
+    if (eventEmitterCallback != null) {
+      eventEmitterCallback.invoke(\\"onEvent2\\", value);
+    }
  }

  protected final void emitOnEvent3(double value) {
-    mEventEmitterCallback.invoke(\\"onEvent3\\", value);
+    CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
+    if (eventEmitterCallback != null) {
+      eventEmitterCallback.invoke(\\"onEvent3\\", value);
+    }
  }

  protected final void emitOnEvent4(boolean value) {
-    mEventEmitterCallback.invoke(\\"onEvent4\\", value);
+    CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
+    if (eventEmitterCallback != null) {
+      eventEmitterCallback.invoke(\\"onEvent4\\", value);
+    }
  }

  protected final void emitOnEvent5(ReadableMap value) {
-    mEventEmitterCallback.invoke(\\"onEvent5\\", value);
+    CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
+    if (eventEmitterCallback != null) {
+      eventEmitterCallback.invoke(\\"onEvent5\\", value);
+    }
  }

  protected final void emitOnEvent6(ReadableArray value) {
-    mEventEmitterCallback.invoke(\\"onEvent6\\", value);
+    CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
+    if (eventEmitterCallback != null) {
+      eventEmitterCallback.invoke(\\"onEvent6\\", value);
+    }
  }

  @ReactMethod
@@ -583,6 +602,7 @@ Map {
 package com.facebook.fbreact.specs;

 import com.facebook.proguard.annotations.DoNotStrip;
+import com.facebook.react.bridge.CxxCallbackImpl;
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.ReactContextBaseJavaModule;
 import com.facebook.react.bridge.ReactMethod;
@@ -602,7 +622,10 @@ public abstract class NativeSampleTurboModuleSpec extends ReactContextBaseJavaMo
  }

  protected final void emitLiteralEvent(String value) {
-    mEventEmitterCallback.invoke(\\"literalEvent\\", value);
+    CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
+    if (eventEmitterCallback != null) {
+      eventEmitterCallback.invoke(\\"literalEvent\\", value);
+    }
  }

  @ReactMethod(isBlockingSynchronousMethod = true)
@@ -326,27 +326,45 @@ Map {
@implementation NativeSampleTurboModuleSpecBase
 - (void)emitOnEvent1
 {
-  _eventEmitterCallback(\\"onEvent1\\", nil);
+  auto eventEmitterCallback = _eventEmitterCallback;
+  if (eventEmitterCallback) {
+    eventEmitterCallback(\\"onEvent1\\", nil);
+  }
 }
 - (void)emitOnEvent2:(NSString *_Nonnull)value
 {
-  _eventEmitterCallback(\\"onEvent2\\", value);
+  auto eventEmitterCallback = _eventEmitterCallback;
+  if (eventEmitterCallback) {
+    eventEmitterCallback(\\"onEvent2\\", value);
+  }
 }
 - (void)emitOnEvent3:(NSNumber *_Nonnull)value
 {
-  _eventEmitterCallback(\\"onEvent3\\", value);
+  auto eventEmitterCallback = _eventEmitterCallback;
+  if (eventEmitterCallback) {
+    eventEmitterCallback(\\"onEvent3\\", value);
+  }
 }
 - (void)emitOnEvent4:(BOOL)value
 {
-  _eventEmitterCallback(\\"onEvent4\\", [NSNumber numberWithBool:value]);
+  auto eventEmitterCallback = _eventEmitterCallback;
+  if (eventEmitterCallback) {
+    eventEmitterCallback(\\"onEvent4\\", [NSNumber numberWithBool:value]);
+  }
 }
 - (void)emitOnEvent5:(NSDictionary *)value
 {
-  _eventEmitterCallback(\\"onEvent5\\", value);
+  auto eventEmitterCallback = _eventEmitterCallback;
+  if (eventEmitterCallback) {
+    eventEmitterCallback(\\"onEvent5\\", value);
+  }
 }
 - (void)emitOnEvent6:(NSArray<id<NSObject>> *)value
 {
-  _eventEmitterCallback(\\"onEvent6\\", value);
+  auto eventEmitterCallback = _eventEmitterCallback;
+  if (eventEmitterCallback) {
+    eventEmitterCallback(\\"onEvent6\\", value);
+  }
 }

 - (void)setEventEmitterCallback:(EventEmitterCallbackWrapper *)eventEmitterCallbackWrapper
@@ -373,8 +391,11 @@ namespace facebook::react {
        eventEmitterMap_[\\"onEvent4\\"] = std::make_shared<AsyncEventEmitter<id>>();
        eventEmitterMap_[\\"onEvent5\\"] = std::make_shared<AsyncEventEmitter<id>>();
        eventEmitterMap_[\\"onEvent6\\"] = std::make_shared<AsyncEventEmitter<id>>();
-        setEventEmitterCallback([&](const std::string &name, id value) {
-          static_cast<AsyncEventEmitter<id> &>(*eventEmitterMap_[name]).emit(value);
+        setEventEmitterCallback([eventEmitterMap = eventEmitterMap_](const std::string &name, id value) {
+          auto it = eventEmitterMap.find(name);
+          if (it != eventEmitterMap.end() && it->second) {
+            static_cast<AsyncEventEmitter<id> &>(*it->second).emit(value);
+          }
        });
  }
 } // namespace facebook::react
@@ -734,7 +755,10 @@ Map {
@implementation NativeSampleTurboModuleSpecBase
 - (void)emitLiteralEvent:(NSString *_Nonnull)value
 {
-  _eventEmitterCallback(\\"literalEvent\\", value);
+  auto eventEmitterCallback = _eventEmitterCallback;
+  if (eventEmitterCallback) {
+    eventEmitterCallback(\\"literalEvent\\", value);
+  }
 }

 - (void)setEventEmitterCallback:(EventEmitterCallbackWrapper *)eventEmitterCallbackWrapper
@@ -756,8 +780,11 @@ namespace facebook::react {
        methodMap_[\\"getStringLiteral\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleSpecJSI_getStringLiteral};
        
        eventEmitterMap_[\\"literalEvent\\"] = std::make_shared<AsyncEventEmitter<id>>();
-        setEventEmitterCallback([&](const std::string &name, id value) {
-          static_cast<AsyncEventEmitter<id> &>(*eventEmitterMap_[name]).emit(value);
+        setEventEmitterCallback([eventEmitterMap = eventEmitterMap_](const std::string &name, id value) {
+          auto it = eventEmitterMap.find(name);
+          if (it != eventEmitterMap.end() && it->second) {
+            static_cast<AsyncEventEmitter<id> &>(*it->second).emit(value);
+          }
        });
  }
 } // namespace facebook::react
@@ -543,6 +543,59 @@ TEST_F(BridgingTest, eventEmitterTest) {
  }
 }

+TEST_F(BridgingTest, eventEmitterMapLookupTest) {
+  // Mirrors the lookup the codegen'd setEventEmitterCallback lambda performs:
+  // an unknown event name must not insert an entry, and a null emitter must not
+  // be dereferenced.
+  std::unordered_map<std::string, std::shared_ptr<IAsyncEventEmitter>>
+      eventEmitterMap;
+  eventEmitterMap["onEvent"] = std::make_shared<AsyncEventEmitter<EventType>>();
+  eventEmitterMap["onNullEvent"] = nullptr;
+
+  // Subscribe to the valid emitter so the emit("onEvent") case can be verified
+  // by an observable side effect, not merely EXPECT_NO_THROW.
+  EventSubscriptionsWithLastEvent eventSubscriptionsWithListener;
+  addEventSubscription<EventType>(
+      rt,
+      static_cast<AsyncEventEmitter<EventType>&>(*eventEmitterMap["onEvent"]),
+      eventSubscriptionsWithListener,
+      invoker);
+
+  auto emit = [&eventEmitterMap](const std::string& name) {
+    auto it = eventEmitterMap.find(name);
+    if (it == eventEmitterMap.end() || !it->second) {
+      return;
+    }
+    static_cast<AsyncEventEmitter<EventType>&>(*it->second)
+        .emit({"one", "two", "three"});
+  };
+
+  EXPECT_NO_THROW(emit("onUnknownEvent"));
+  EXPECT_NO_THROW(emit("onNullEvent"));
+  EXPECT_NO_THROW(emit("onEvent"));
+
+  // The valid emit must actually reach the subscribed listener.
+  flushQueue();
+  ASSERT_EQ(1, eventSubscriptionsWithListener.size());
+  const auto& lastEvent = eventSubscriptionsWithListener[0].second;
+  ASSERT_EQ(3, lastEvent->size());
+  EXPECT_EQ("one", lastEvent->at(0));
+  EXPECT_EQ("two", lastEvent->at(1));
+  EXPECT_EQ("three", lastEvent->at(2));
+
+  // The miss must not have grown the map, which operator[] would have done.
+  EXPECT_EQ(2, eventEmitterMap.size());
+  EXPECT_FALSE(eventEmitterMap.contains("onUnknownEvent"));
+
+  // Clean up the subscription so the LongLivedObjectCollection leak check in
+  // TearDown passes.
+  for (const auto& [eventSubscription, _] : eventSubscriptionsWithListener) {
+    eventSubscription.getPropertyAsFunction(rt, "remove")
+        .callWithThis(rt, eventSubscription);
+  }
+  flushQueue();
+}
+
 TEST_F(BridgingTest, optionalTest) {
  EXPECT_EQ(
      1, bridging::fromJs<std::optional<int>>(rt, jsi::Value(1), invoker));
@@ -1054,12 +1054,20 @@ void JavaTurboModule::configureEventEmitterCallback() {
    FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
  }

-  auto callback = JCxxCallbackImpl::newObjectCxxArgs([&](folly::dynamic args) {
-    auto eventName = args.at(0).asString();
-    auto& eventEmitter = static_cast<AsyncEventEmitter<folly::dynamic>&>(
-        *eventEmitterMap_[eventName].get());
-    eventEmitter.emit(args.size() > 1 ? std::move(args).at(1) : nullptr);
-  });
+  // The Java module owns the callback and can outlive this object, so the
+  // lambda captures its own copy of the map rather than referencing
+  // eventEmitterMap_. Callers register every emitter before reaching here;
+  // emitters added afterwards are not visible to this callback.
+  auto callback = JCxxCallbackImpl::newObjectCxxArgs(
+      [eventEmitterMap = eventEmitterMap_](folly::dynamic args) {
+        auto eventName = args.at(0).asString();
+        auto it = eventEmitterMap.find(eventName);
+        if (it == eventEmitterMap.end() || !it->second) {
+          return;
+        }
+        static_cast<AsyncEventEmitter<folly::dynamic>&>(*it->second)
+            .emit(args.size() > 1 ? std::move(args).at(1) : nullptr);
+      });

  jvalue args[1];
  args[0].l = callback.release();