backport fix for 32 bit android to 0.79

[mljlynch]

Changelog:

[ANDROID] [FIXED] - (#51628) emitting event from turbo module crashes on 32bit android in 0.79

Original commit message:

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.

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, install the dependencies (v 0.79 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.

[trcoffman]

Please merge this since Expo does not support React Native 0.80

[mljlynch]

Please merge this since Expo does not support React Native 0.80

@trcoffman you can fix by applying as patch as suggested in the related issue:

patches/react-native+0.79.4.patch

diff --git a/node_modules/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/node_modules/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp
index b36755e..cba8948 100644
--- a/node_modules/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp
+++ b/node_modules/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp
@@ -989,15 +989,19 @@ void JavaTurboModule::setEventEmitterCallback(
         *eventEmitterMap_[eventName].get());
   };

-  jvalue arg;
-  arg.l = JCxxCallbackImpl::newObjectCxxArgs([eventEmitterLookup = std::move(
+  // Patch from https://github.com/facebook/react-native/pull/51695 to fix https://github.com/facebook/react-native/issues/51628#issuecomment-2922391662. Not needed if https://github.com/facebook/react-native/pull/52377 is merged and a patch version for 0.79 is released
+  auto callback = JCxxCallbackImpl::newObjectCxxArgs([eventEmitterLookup = std::move(
                                                   eventEmitterLookup)](
                                                  folly::dynamic args) {
             auto eventName = args.at(0).asString();
             auto eventArgs = args.size() > 1 ? args.at(1) : nullptr;
             eventEmitterLookup(eventName).emit(std::move(eventArgs));
-          }).release();
-  env->CallVoidMethod(instance, cachedMethodId, arg);
+          });
+  jvalue args[1];
+  args[0].l = callback.release();
+  // CallVoidMethod is replaced with CallVoidMethodA as it's unsafe on 32bit and causes crashes
+  // https://github.com/facebook/react-native/issues/51628
+  env->CallVoidMethodA(instance, cachedMethodId, args);
 }

 } // namespace facebook::react

[trcoffman]

@mljlynch Unfortunately, we cannot build react-native from source due to jpudysz/react-native-unistyles#897

Also, building from source is not that simple for expo apps leveraging CNG. You have to custom build a config plugin to do this because there is no android directory checked into version control, it gets generated at build time.

I think that an instant crash on startup that affects many devices should absolutely be packported to 0.79 considering that Expo is not supporting 0.80 in SDK 53.

[jleprinc]

I'm in the same case as trcoffman, I cannot build from source and I need this fix as it's creating many crashs in production. Upgrading to react native 0.80 is not an option as expo 54 is still in beta. This patch has been approved by many react native developers, I think it would be safe to merge it.

[martinnovak22]

Same as others we are having issues with this and can't upgrade right now because of expo. Any updates on this? Will it be merged to 0.79?

[markwilcox]

It's far from ideal that we have a version that basically doesn't work on 32-bit Android devices, however as stated on other issues this patch is a breaking change and can't be a solution for the 0.79 series. I can confirm having tried the patch that it causes build failures with some other native modules.

So if it works for your app patch it. We're currently branched old architecture for Android, new architecture for iOS.

[mljlynch]

Closing as this was ported into 0.79.6