Skip to content

Commit 43bc43e

Browse files
vladimirivanovilievcipolleschi
authored andcommitted
Fix emitting event from turbo module crashes on 32bit android (#51695)
Summary: After testing the latest RC and nighly builds, crash appeared when emitting events from turbo modules on 32bit Android devices. The crash is always reproducible only on 32bit devices on signed production builds. Fore more details and the crash log, check the [related issue](#51628 (comment)). From what I found, the variadic functions like CallVoidMethod are unsafe on 32bit due to not type checking the passed arguments at compile time. As far as I understand the 64bit cpus and ABIs are more forgiving with alignment and calling conventions. On 32bit the ABIs are strict as arguments are passed on the stack and if there is type/size/alignment issue it reads the wrong memory, which causes the SIGEGV crashes. [ANDROID] [FIXED] - emitting event from turbo module crashes on 32bit android Pull Request resolved: #51695 Test Plan: 1. Pull the [reproduction demo](https://github.com/vladimirivanoviliev/rn079eventcrash), install the dependencies (v `0.80` is on PR) 2. Run codegen on android 3. Build signed apk. To create it you will need to create new demo key-store. 4. To install the build apk in 32bit mode you can use `adb -s YOURDEVICE install --abi armeabi-v7a android/app/release/app-release.apk` 5. Run the app, create key, save it. Than update the key and save it again. The app crashes when try to emit event from the turbo module. 6. Patch the related `JavaTurboModule.cpp` file with the changes from this PR and enable build from source. 7. Rebuild and reinstall the apk and test again - the issue is now fixed I have tested the app on android using the `rn-tester` demo app, everything works as expected. I also patched our production app and tested more complex scenarios and they works as expected. I have run the tests and linter and they passed. One thing that I didn't able to setup and run is the iOS `rn-tester` app, due to Hermes engine error `Command PhaseScriptExecution failed with a nonzero exit code`. I haven't found any information how to fix it. I have followed [this guide](https://github.com/facebook/react-native/blob/main/packages/rn-tester/README.md) and installed node modules using yarn and started the `yarn prepare-ios`. I also haven't found any information with what node version and ruby version the react native package is build on CI so I use the same versions locally. If you provide me with updated instructions for those I can contribute by updating the related guides and including `.npmrc`, `.ruby-version` files. Reviewed By: cortinico Differential Revision: D75782377 Pulled By: javache fbshipit-source-id: b94998be6dd51e90ad4137b1d2e38a6850bc3cb2
1 parent d97f6f6 commit 43bc43e

File tree

1 file changed

+12
-8
lines changed
  • packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon

1 file changed

+12
-8
lines changed

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -989,15 +989,19 @@ void JavaTurboModule::setEventEmitterCallback(
989989
*eventEmitterMap_[eventName].get());
990990
};
991991

992-
jvalue arg;
993-
arg.l = JCxxCallbackImpl::newObjectCxxArgs([eventEmitterLookup = std::move(
994-
eventEmitterLookup)](
992+
auto callback = JCxxCallbackImpl::newObjectCxxArgs([eventEmitterLookup = std::move(eventEmitterLookup)](
995993
folly::dynamic args) {
996-
auto eventName = args.at(0).asString();
997-
auto eventArgs = args.size() > 1 ? args.at(1) : nullptr;
998-
eventEmitterLookup(eventName).emit(std::move(eventArgs));
999-
}).release();
1000-
env->CallVoidMethod(instance, cachedMethodId, arg);
994+
auto eventName = args.at(0).asString();
995+
auto eventArgs = args.size() > 1 ? args.at(1) : nullptr;
996+
eventEmitterLookup(eventName).emit(std::move(eventArgs));
997+
});
998+
999+
jvalue args[1];
1000+
args[0].l = callback.release();
1001+
// CallVoidMethod is replaced with CallVoidMethodA as it's unsafe on 32bit and
1002+
// causes crashes https://github.com/facebook/react-native/issues/51628
1003+
env->CallVoidMethodA(instance_.get(), cachedMethodId, args);
1004+
FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
10011005
}
10021006

10031007
} // namespace facebook::react

0 commit comments

Comments
 (0)