diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js index efb710aaae0a..42088e4f0990 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleJavaSpec.js @@ -82,16 +82,25 @@ function EventEmitterTemplate( eventEmitter: NativeModuleEventEmitterShape, imports: Set, ): 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' + : '' + }); + } }`; } diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeEventEmitter.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeEventEmitter.js index 72582c12bf7c..b5ea25048fef 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeEventEmitter.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeEventEmitter.js @@ -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' + }); + } }`; } diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/source/serializeModule.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/source/serializeModule.js index 8898f3294720..371f640fb82b 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/source/serializeModule.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/source/serializeModule.js @@ -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 &>(*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 &>(*it->second).emit(value); + } });` : '' } diff --git a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJavaSpec-test.js.snap b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJavaSpec-test.js.snap index a5a124961180..02bf916002af 100644 --- a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJavaSpec-test.js.snap +++ b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJavaSpec-test.js.snap @@ -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) diff --git a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleMm-test.js.snap b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleMm-test.js.snap index c2b1cf853c45..f70403ac89b6 100644 --- a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleMm-test.js.snap +++ b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleMm-test.js.snap @@ -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> *)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>(); eventEmitterMap_[\\"onEvent5\\"] = std::make_shared>(); eventEmitterMap_[\\"onEvent6\\"] = std::make_shared>(); - setEventEmitterCallback([&](const std::string &name, id value) { - static_cast &>(*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 &>(*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>(); - setEventEmitterCallback([&](const std::string &name, id value) { - static_cast &>(*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 &>(*it->second).emit(value); + } }); } } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/bridging/tests/BridgingTest.cpp b/packages/react-native/ReactCommon/react/bridging/tests/BridgingTest.cpp index cec7c92657c8..d26496fd58fb 100644 --- a/packages/react-native/ReactCommon/react/bridging/tests/BridgingTest.cpp +++ b/packages/react-native/ReactCommon/react/bridging/tests/BridgingTest.cpp @@ -542,6 +542,33 @@ 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> + eventEmitterMap; + eventEmitterMap["onEvent"] = std::make_shared>(); + eventEmitterMap["onNullEvent"] = nullptr; + + auto emit = [&eventEmitterMap](const std::string& name) { + auto it = eventEmitterMap.find(name); + if (it == eventEmitterMap.end() || !it->second) { + return; + } + static_cast&>(*it->second) + .emit({"one", "two", "three"}); + }; + + EXPECT_NO_THROW(emit("onUnknownEvent")); + EXPECT_NO_THROW(emit("onNullEvent")); + EXPECT_NO_THROW(emit("onEvent")); + + // The miss must not have grown the map, which operator[] would have done. + EXPECT_EQ(2, eventEmitterMap.size()); + EXPECT_FALSE(eventEmitterMap.contains("onUnknownEvent")); +} + TEST_F(BridgingTest, optionalTest) { EXPECT_EQ( 1, bridging::fromJs>(rt, jsi::Value(1), invoker)); diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index cec9eaf5f02d..4970f6a66acc 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -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&>( - *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&>(*it->second) + .emit(args.size() > 1 ? std::move(args).at(1) : nullptr); + }); jvalue args[1]; args[0].l = callback.release();