Skip to content

Commit ac09dbb

Browse files
Make codegen'd TurboModule event emitters no-op when the emitter callback is absent (#57893)
Summary: 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 Differential Revision: D115130767
1 parent f4803df commit ac09dbb

7 files changed

Lines changed: 148 additions & 39 deletions

File tree

packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,16 +82,25 @@ function EventEmitterTemplate(
8282
eventEmitter: NativeModuleEventEmitterShape,
8383
imports: Set<string>,
8484
): string {
85+
imports.add('com.facebook.react.bridge.CxxCallbackImpl');
86+
// mEventEmitterCallback is set from JNI by configureEventEmitterCallback(),
87+
// which the generated SpecJSI constructor calls when JS first looks the module
88+
// up. Emitting before that would hit a null field, so the emitted code no-ops
89+
// instead. The local is for readability: reference reads are already atomic,
90+
// and the field is never reset to null.
8591
return ` protected final void emit${toPascalCase(eventEmitter.name)}(${
8692
eventEmitter.typeAnnotation.typeAnnotation.type !== 'VoidTypeAnnotation'
8793
? `${translateEventEmitterTypeToJavaType(eventEmitter, imports)} value`
8894
: ''
8995
}) {
90-
mEventEmitterCallback.invoke("${eventEmitter.name}"${
91-
eventEmitter.typeAnnotation.typeAnnotation.type !== 'VoidTypeAnnotation'
92-
? ', value'
93-
: ''
94-
});
96+
CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
97+
if (eventEmitterCallback != null) {
98+
eventEmitterCallback.invoke("${eventEmitter.name}"${
99+
eventEmitter.typeAnnotation.typeAnnotation.type !== 'VoidTypeAnnotation'
100+
? ', value'
101+
: ''
102+
});
103+
}
95104
}`;
96105
}
97106

packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeEventEmitter.js

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,20 +78,28 @@ function EventEmitterHeaderTemplate(
7878
function EventEmitterImplementationTemplate(
7979
eventEmitter: NativeModuleEventEmitterShape,
8080
): string {
81+
// _eventEmitterCallback is installed by the generated SpecJSI constructor,
82+
// which runs when JS first looks the module up. Emitting before that would
83+
// call an empty std::function, so the emitted code no-ops instead. The local
84+
// copy is for readability; it does not synchronize against a concurrent
85+
// install.
8186
return `- (void)emit${toPascalCase(eventEmitter.name)}${
8287
eventEmitter.typeAnnotation.typeAnnotation.type !== 'VoidTypeAnnotation'
8388
? `:(${getEventEmitterTypeObjCType(eventEmitter)})value`
8489
: ''
8590
}
8691
{
87-
_eventEmitterCallback("${eventEmitter.name}", ${
88-
eventEmitter.typeAnnotation.typeAnnotation.type !== 'VoidTypeAnnotation'
89-
? eventEmitter.typeAnnotation.typeAnnotation.type !==
90-
'BooleanTypeAnnotation'
91-
? 'value'
92-
: '[NSNumber numberWithBool:value]'
93-
: 'nil'
94-
});
92+
auto eventEmitterCallback = _eventEmitterCallback;
93+
if (eventEmitterCallback) {
94+
eventEmitterCallback("${eventEmitter.name}", ${
95+
eventEmitter.typeAnnotation.typeAnnotation.type !== 'VoidTypeAnnotation'
96+
? eventEmitter.typeAnnotation.typeAnnotation.type !==
97+
'BooleanTypeAnnotation'
98+
? 'value'
99+
: '[NSNumber numberWithBool:value]'
100+
: 'nil'
101+
});
102+
}
95103
}`;
96104
}
97105

packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/source/serializeModule.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,17 @@ namespace facebook::react {
8383
.join('')
8484
: ''
8585
}${
86+
// The callback outlives this module, so it captures a copy of the map
87+
// instead of referencing eventEmitterMap_. Every emitter is registered
88+
// directly above, so the copy is complete; an emitter registered after
89+
// construction would not be reachable from the callback.
8690
eventEmitters.length > 0
8791
? `
88-
setEventEmitterCallback([&](const std::string &name, id value) {
89-
static_cast<AsyncEventEmitter<id> &>(*eventEmitterMap_[name]).emit(value);
92+
setEventEmitterCallback([eventEmitterMap = eventEmitterMap_](const std::string &name, id value) {
93+
auto it = eventEmitterMap.find(name);
94+
if (it != eventEmitterMap.end() && it->second) {
95+
static_cast<AsyncEventEmitter<id> &>(*it->second).emit(value);
96+
}
9097
});`
9198
: ''
9299
}

packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJavaSpec-test.js.snap

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ Map {
229229
package com.facebook.fbreact.specs;
230230
231231
import com.facebook.proguard.annotations.DoNotStrip;
232+
import com.facebook.react.bridge.CxxCallbackImpl;
232233
import com.facebook.react.bridge.ReactApplicationContext;
233234
import com.facebook.react.bridge.ReactContextBaseJavaModule;
234235
import com.facebook.react.bridge.ReactMethod;
@@ -250,27 +251,45 @@ public abstract class NativeSampleTurboModuleSpec extends ReactContextBaseJavaMo
250251
}
251252
252253
protected final void emitOnEvent1() {
253-
mEventEmitterCallback.invoke(\\"onEvent1\\");
254+
CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
255+
if (eventEmitterCallback != null) {
256+
eventEmitterCallback.invoke(\\"onEvent1\\");
257+
}
254258
}
255259
256260
protected final void emitOnEvent2(String value) {
257-
mEventEmitterCallback.invoke(\\"onEvent2\\", value);
261+
CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
262+
if (eventEmitterCallback != null) {
263+
eventEmitterCallback.invoke(\\"onEvent2\\", value);
264+
}
258265
}
259266
260267
protected final void emitOnEvent3(double value) {
261-
mEventEmitterCallback.invoke(\\"onEvent3\\", value);
268+
CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
269+
if (eventEmitterCallback != null) {
270+
eventEmitterCallback.invoke(\\"onEvent3\\", value);
271+
}
262272
}
263273
264274
protected final void emitOnEvent4(boolean value) {
265-
mEventEmitterCallback.invoke(\\"onEvent4\\", value);
275+
CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
276+
if (eventEmitterCallback != null) {
277+
eventEmitterCallback.invoke(\\"onEvent4\\", value);
278+
}
266279
}
267280
268281
protected final void emitOnEvent5(ReadableMap value) {
269-
mEventEmitterCallback.invoke(\\"onEvent5\\", value);
282+
CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
283+
if (eventEmitterCallback != null) {
284+
eventEmitterCallback.invoke(\\"onEvent5\\", value);
285+
}
270286
}
271287
272288
protected final void emitOnEvent6(ReadableArray value) {
273-
mEventEmitterCallback.invoke(\\"onEvent6\\", value);
289+
CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
290+
if (eventEmitterCallback != null) {
291+
eventEmitterCallback.invoke(\\"onEvent6\\", value);
292+
}
274293
}
275294
276295
@ReactMethod
@@ -583,6 +602,7 @@ Map {
583602
package com.facebook.fbreact.specs;
584603
585604
import com.facebook.proguard.annotations.DoNotStrip;
605+
import com.facebook.react.bridge.CxxCallbackImpl;
586606
import com.facebook.react.bridge.ReactApplicationContext;
587607
import com.facebook.react.bridge.ReactContextBaseJavaModule;
588608
import com.facebook.react.bridge.ReactMethod;
@@ -602,7 +622,10 @@ public abstract class NativeSampleTurboModuleSpec extends ReactContextBaseJavaMo
602622
}
603623
604624
protected final void emitLiteralEvent(String value) {
605-
mEventEmitterCallback.invoke(\\"literalEvent\\", value);
625+
CxxCallbackImpl eventEmitterCallback = mEventEmitterCallback;
626+
if (eventEmitterCallback != null) {
627+
eventEmitterCallback.invoke(\\"literalEvent\\", value);
628+
}
606629
}
607630
608631
@ReactMethod(isBlockingSynchronousMethod = true)

packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleMm-test.js.snap

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -326,27 +326,45 @@ Map {
326326
@implementation NativeSampleTurboModuleSpecBase
327327
- (void)emitOnEvent1
328328
{
329-
_eventEmitterCallback(\\"onEvent1\\", nil);
329+
auto eventEmitterCallback = _eventEmitterCallback;
330+
if (eventEmitterCallback) {
331+
eventEmitterCallback(\\"onEvent1\\", nil);
332+
}
330333
}
331334
- (void)emitOnEvent2:(NSString *_Nonnull)value
332335
{
333-
_eventEmitterCallback(\\"onEvent2\\", value);
336+
auto eventEmitterCallback = _eventEmitterCallback;
337+
if (eventEmitterCallback) {
338+
eventEmitterCallback(\\"onEvent2\\", value);
339+
}
334340
}
335341
- (void)emitOnEvent3:(NSNumber *_Nonnull)value
336342
{
337-
_eventEmitterCallback(\\"onEvent3\\", value);
343+
auto eventEmitterCallback = _eventEmitterCallback;
344+
if (eventEmitterCallback) {
345+
eventEmitterCallback(\\"onEvent3\\", value);
346+
}
338347
}
339348
- (void)emitOnEvent4:(BOOL)value
340349
{
341-
_eventEmitterCallback(\\"onEvent4\\", [NSNumber numberWithBool:value]);
350+
auto eventEmitterCallback = _eventEmitterCallback;
351+
if (eventEmitterCallback) {
352+
eventEmitterCallback(\\"onEvent4\\", [NSNumber numberWithBool:value]);
353+
}
342354
}
343355
- (void)emitOnEvent5:(NSDictionary *)value
344356
{
345-
_eventEmitterCallback(\\"onEvent5\\", value);
357+
auto eventEmitterCallback = _eventEmitterCallback;
358+
if (eventEmitterCallback) {
359+
eventEmitterCallback(\\"onEvent5\\", value);
360+
}
346361
}
347362
- (void)emitOnEvent6:(NSArray<id<NSObject>> *)value
348363
{
349-
_eventEmitterCallback(\\"onEvent6\\", value);
364+
auto eventEmitterCallback = _eventEmitterCallback;
365+
if (eventEmitterCallback) {
366+
eventEmitterCallback(\\"onEvent6\\", value);
367+
}
350368
}
351369
352370
- (void)setEventEmitterCallback:(EventEmitterCallbackWrapper *)eventEmitterCallbackWrapper
@@ -373,8 +391,11 @@ namespace facebook::react {
373391
eventEmitterMap_[\\"onEvent4\\"] = std::make_shared<AsyncEventEmitter<id>>();
374392
eventEmitterMap_[\\"onEvent5\\"] = std::make_shared<AsyncEventEmitter<id>>();
375393
eventEmitterMap_[\\"onEvent6\\"] = std::make_shared<AsyncEventEmitter<id>>();
376-
setEventEmitterCallback([&](const std::string &name, id value) {
377-
static_cast<AsyncEventEmitter<id> &>(*eventEmitterMap_[name]).emit(value);
394+
setEventEmitterCallback([eventEmitterMap = eventEmitterMap_](const std::string &name, id value) {
395+
auto it = eventEmitterMap.find(name);
396+
if (it != eventEmitterMap.end() && it->second) {
397+
static_cast<AsyncEventEmitter<id> &>(*it->second).emit(value);
398+
}
378399
});
379400
}
380401
} // namespace facebook::react
@@ -734,7 +755,10 @@ Map {
734755
@implementation NativeSampleTurboModuleSpecBase
735756
- (void)emitLiteralEvent:(NSString *_Nonnull)value
736757
{
737-
_eventEmitterCallback(\\"literalEvent\\", value);
758+
auto eventEmitterCallback = _eventEmitterCallback;
759+
if (eventEmitterCallback) {
760+
eventEmitterCallback(\\"literalEvent\\", value);
761+
}
738762
}
739763
740764
- (void)setEventEmitterCallback:(EventEmitterCallbackWrapper *)eventEmitterCallbackWrapper
@@ -756,8 +780,11 @@ namespace facebook::react {
756780
methodMap_[\\"getStringLiteral\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleSpecJSI_getStringLiteral};
757781
758782
eventEmitterMap_[\\"literalEvent\\"] = std::make_shared<AsyncEventEmitter<id>>();
759-
setEventEmitterCallback([&](const std::string &name, id value) {
760-
static_cast<AsyncEventEmitter<id> &>(*eventEmitterMap_[name]).emit(value);
783+
setEventEmitterCallback([eventEmitterMap = eventEmitterMap_](const std::string &name, id value) {
784+
auto it = eventEmitterMap.find(name);
785+
if (it != eventEmitterMap.end() && it->second) {
786+
static_cast<AsyncEventEmitter<id> &>(*it->second).emit(value);
787+
}
761788
});
762789
}
763790
} // namespace facebook::react

packages/react-native/ReactCommon/react/bridging/tests/BridgingTest.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,33 @@ TEST_F(BridgingTest, eventEmitterTest) {
542542
}
543543
}
544544

545+
TEST_F(BridgingTest, eventEmitterMapLookupTest) {
546+
// Mirrors the lookup the codegen'd setEventEmitterCallback lambda performs:
547+
// an unknown event name must not insert an entry, and a null emitter must not
548+
// be dereferenced.
549+
std::unordered_map<std::string, std::shared_ptr<IAsyncEventEmitter>>
550+
eventEmitterMap;
551+
eventEmitterMap["onEvent"] = std::make_shared<AsyncEventEmitter<EventType>>();
552+
eventEmitterMap["onNullEvent"] = nullptr;
553+
554+
auto emit = [&eventEmitterMap](const std::string& name) {
555+
auto it = eventEmitterMap.find(name);
556+
if (it == eventEmitterMap.end() || !it->second) {
557+
return;
558+
}
559+
static_cast<AsyncEventEmitter<EventType>&>(*it->second)
560+
.emit({"one", "two", "three"});
561+
};
562+
563+
EXPECT_NO_THROW(emit("onUnknownEvent"));
564+
EXPECT_NO_THROW(emit("onNullEvent"));
565+
EXPECT_NO_THROW(emit("onEvent"));
566+
567+
// The miss must not have grown the map, which operator[] would have done.
568+
EXPECT_EQ(2, eventEmitterMap.size());
569+
EXPECT_FALSE(eventEmitterMap.contains("onUnknownEvent"));
570+
}
571+
545572
TEST_F(BridgingTest, optionalTest) {
546573
EXPECT_EQ(
547574
1, bridging::fromJs<std::optional<int>>(rt, jsi::Value(1), invoker));

packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,12 +1054,20 @@ void JavaTurboModule::configureEventEmitterCallback() {
10541054
FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
10551055
}
10561056

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

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

0 commit comments

Comments
 (0)