Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update jsc-android to ndk r23 based #36062

Closed
wants to merge 2 commits into from
Closed

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Feb 5, 2023

Summary

the current jsc-android is still built based on ndk r21, and react-native is now built based on ndk r23. the unwinder between r21 and r23 is incompatible (libgcc vs libunwind). if there's exceptions throwing from jsc, other react native libraries cannot catch these exceptions and cause runtime crash.

this pr updates jsc-android to 235231.0.0 which is the same webkitgtk version as 235230.2.1 but only built by ndk r23. the jsc-android pr is from react-native-community/jsc-android-buildscripts#179. note that the jsc is based on ndk r23c and react-native is based on ndk r23b. the reason is that i cannot get jsc building successfully on r23b. hopefully r23b and r23c are abi safe.

there is another crash from libjscexecutor when testing the new jsc-android. to fix the issue, i have to explicitly link libunwind.a from libjscexecutor.so. supposedly ndk r23 should help to link libunwind under the hood, i still not figure out why it doesn't. but after linking libunwind.a, i can get new jsc-android work successfully.

E/art     ( 2669): dlopen("/data/app/com.test-1/lib/x86_64/libjscexecutor.so", RTLD_LAZY) failed: dlopen failed: cannot locate symbol "_Unwind_Resume" referenced by "/data/app/com.test-1/lib/x86_64/libjscexecutor.so"...
W/System.err( 2669): java.lang.UnsatisfiedLinkError: dlopen failed: cannot locate symbol "_Unwind_Resume" referenced by "/data/app/com.test-1/lib/x86_64/libjscexecutor.so"...
W/System.err( 2669): 	at java.lang.Runtime.load(Runtime.java:331)
W/System.err( 2669): 	at java.lang.System.load(System.java:982)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader$1.load(SoLoader.java:558)
W/System.err( 2669): 	at com.facebook.soloader.DirectorySoSource.loadLibraryFrom(DirectorySoSource.java:110)
W/System.err( 2669): 	at com.facebook.soloader.DirectorySoSource.loadLibrary(DirectorySoSource.java:63)
W/System.err( 2669): 	at com.facebook.soloader.ApplicationSoSource.loadLibrary(ApplicationSoSource.java:91)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.doLoadLibraryBySoName(SoLoader.java:1067)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.loadLibraryBySoNameImpl(SoLoader.java:943)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.loadLibraryBySoName(SoLoader.java:855)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.loadLibrary(SoLoader.java:802)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.loadLibrary(SoLoader.java:772)
W/System.err( 2669): 	at com.facebook.react.jscexecutor.JSCExecutor.loadLibrary(JSCExecutor.java:24)
W/System.err( 2669): 	at com.facebook.react.jscexecutor.JSCExecutor.<clinit>(JSCExecutor.java:20)
W/System.err( 2669): 	at com.facebook.react.ReactInstanceManagerBuilder.getDefaultJSExecutorFactory(ReactInstanceManagerBuilder.java:363)
W/System.err( 2669): 	at com.facebook.react.ReactInstanceManagerBuilder.build(ReactInstanceManagerBuilder.java:316)
W/System.err( 2669): 	at com.facebook.react.ReactNativeHost.createReactInstanceManager(ReactNativeHost.java:94)
W/System.err( 2669): 	at com.facebook.react.ReactNativeHost.getReactInstanceManager(ReactNativeHost.java:41)
W/System.err( 2669): 	at com.test.MainApplication.onCreate(MainApplication.java:60)
W/System.err( 2669): 	at android.app.Instrumentation.callApplicationOnCreate(Instrumentation.java:1011)
W/System.err( 2669): 	at androidx.test.runner.MonitoringInstrumentation.callApplicationOnCreate(MonitoringInstrumentation.java:483)
W/System.err( 2669): 	at android.app.ActivityThread.handleBindApplication(ActivityThread.java:4518)
W/System.err( 2669): 	at android.app.ActivityThread.access$1500(ActivityThread.java:144)
W/System.err( 2669): 	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1339)
W/System.err( 2669): 	at android.os.Handler.dispatchMessage(Handler.java:102)
W/System.err( 2669): 	at android.os.Looper.loop(Looper.java:135)
W/System.err( 2669): 	at android.app.ActivityThread.main(ActivityThread.java:5221)
W/System.err( 2669): 	at java.lang.reflect.Method.invoke(Native Method)
W/System.err( 2669): 	at java.lang.reflect.Method.invoke(Method.java:372)
W/System.err( 2669): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:899)
W/System.err( 2669): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:694)

fixes #36052

Changelog

[ANDROID][FIXED] - Fixed jscexecutor crash on Android which is caused from NDK incompatibility

Test Plan

tested on jsc-android instrumented test (based on react-native 0.71.2)

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. p: Expo Partner: Expo Partner labels Feb 5, 2023
@github-actions
Copy link

github-actions bot commented Feb 5, 2023

Warnings
⚠️ 🔒 package.json - Changes were made to package.json. This will require a manual import by a Facebook employee.

Generated by 🚫 dangerJS against 816778d

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,474,289 +0
android hermes armeabi-v7a 7,795,948 +0
android hermes x86 8,950,179 +0
android hermes x86_64 8,807,843 +0
android jsc arm64-v8a 9,112,211 -544,587
android jsc armeabi-v7a 8,309,054 -82,987
android jsc x86 9,163,725 -556,695
android jsc x86_64 9,422,398 -774,682

Base commit: 7f2dd1d
Branch: main

@kelset kelset requested a review from cortinico February 6, 2023 09:56
@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@cortinico
Copy link
Contributor

This will take a bit longer than usual to build as our internal environment where we're testing the OSS setup is missing libunwind. I'm working on sorting this out

@Kudo
Copy link
Contributor Author

Kudo commented Feb 6, 2023

fwiw, libunwind is introduced by default after ndk r23. before r23, ndk uses libgcc's implementation. we used to have a workaround to link libgcc. maybe the problem depends on the unwinder implementation in the custom ndk meta used internal.

@cortinico
Copy link
Contributor

fwiw, libunwind is introduced by default after ndk r23. before r23, ndk uses libgcc's implementation. we used to have a workaround to link libgcc. maybe the problem depends on the unwinder implementation in the custom ndk meta used internal.

Thanks for sharing this as this unblocked me! I was able to code the same workaround of our setup (our infra is on a older version of the NDK).

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Feb 7, 2023
@facebook-github-bot
Copy link
Contributor

@cortinico merged this pull request in a232dec.

cipolleschi pushed a commit that referenced this pull request Feb 13, 2023
Summary:
the current jsc-android is still built based on ndk r21, and react-native is now built based on ndk r23. the unwinder between r21 and r23 is incompatible (libgcc vs libunwind). if there's exceptions throwing from jsc, other react native libraries cannot catch these exceptions and cause runtime crash.

this pr updates jsc-android to 235231.0.0 which is the same webkitgtk version as 235230.2.1 but only built by ndk r23. the jsc-android pr is from react-native-community/jsc-android-buildscripts#179. note that the jsc is based on ndk r23c and react-native is based on ndk r23b. the reason is that i cannot get jsc building successfully on r23b. hopefully r23b and r23c are abi safe.

there is another crash from libjscexecutor when testing the new jsc-android. to fix the issue, i have to explicitly link libunwind.a from libjscexecutor.so. supposedly ndk r23 should help to link libunwind under the hood, i still not figure out why it doesn't. but after linking libunwind.a, i can get new jsc-android work successfully.

```
E/art     ( 2669): dlopen("/data/app/com.test-1/lib/x86_64/libjscexecutor.so", RTLD_LAZY) failed: dlopen failed: cannot locate symbol "_Unwind_Resume" referenced by "/data/app/com.test-1/lib/x86_64/libjscexecutor.so"...
W/System.err( 2669): java.lang.UnsatisfiedLinkError: dlopen failed: cannot locate symbol "_Unwind_Resume" referenced by "/data/app/com.test-1/lib/x86_64/libjscexecutor.so"...
W/System.err( 2669): 	at java.lang.Runtime.load(Runtime.java:331)
W/System.err( 2669): 	at java.lang.System.load(System.java:982)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader$1.load(SoLoader.java:558)
W/System.err( 2669): 	at com.facebook.soloader.DirectorySoSource.loadLibraryFrom(DirectorySoSource.java:110)
W/System.err( 2669): 	at com.facebook.soloader.DirectorySoSource.loadLibrary(DirectorySoSource.java:63)
W/System.err( 2669): 	at com.facebook.soloader.ApplicationSoSource.loadLibrary(ApplicationSoSource.java:91)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.doLoadLibraryBySoName(SoLoader.java:1067)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.loadLibraryBySoNameImpl(SoLoader.java:943)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.loadLibraryBySoName(SoLoader.java:855)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.loadLibrary(SoLoader.java:802)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.loadLibrary(SoLoader.java:772)
W/System.err( 2669): 	at com.facebook.react.jscexecutor.JSCExecutor.loadLibrary(JSCExecutor.java:24)
W/System.err( 2669): 	at com.facebook.react.jscexecutor.JSCExecutor.<clinit>(JSCExecutor.java:20)
W/System.err( 2669): 	at com.facebook.react.ReactInstanceManagerBuilder.getDefaultJSExecutorFactory(ReactInstanceManagerBuilder.java:363)
W/System.err( 2669): 	at com.facebook.react.ReactInstanceManagerBuilder.build(ReactInstanceManagerBuilder.java:316)
W/System.err( 2669): 	at com.facebook.react.ReactNativeHost.createReactInstanceManager(ReactNativeHost.java:94)
W/System.err( 2669): 	at com.facebook.react.ReactNativeHost.getReactInstanceManager(ReactNativeHost.java:41)
W/System.err( 2669): 	at com.test.MainApplication.onCreate(MainApplication.java:60)
W/System.err( 2669): 	at android.app.Instrumentation.callApplicationOnCreate(Instrumentation.java:1011)
W/System.err( 2669): 	at androidx.test.runner.MonitoringInstrumentation.callApplicationOnCreate(MonitoringInstrumentation.java:483)
W/System.err( 2669): 	at android.app.ActivityThread.handleBindApplication(ActivityThread.java:4518)
W/System.err( 2669): 	at android.app.ActivityThread.access$1500(ActivityThread.java:144)
W/System.err( 2669): 	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1339)
W/System.err( 2669): 	at android.os.Handler.dispatchMessage(Handler.java:102)
W/System.err( 2669): 	at android.os.Looper.loop(Looper.java:135)
W/System.err( 2669): 	at android.app.ActivityThread.main(ActivityThread.java:5221)
W/System.err( 2669): 	at java.lang.reflect.Method.invoke(Native Method)
W/System.err( 2669): 	at java.lang.reflect.Method.invoke(Method.java:372)
W/System.err( 2669): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:899)
W/System.err( 2669): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:694)
```

fixes #36052

## Changelog

[ANDROID][FIXED] - Fixed jscexecutor crash on Android which is caused from NDK incompatibility

Pull Request resolved: #36062

Test Plan: tested on [jsc-android instrumented test](https://github.com/react-native-community/jsc-android-buildscripts/tree/2.26.1/test) (based on react-native 0.71.2)

Reviewed By: cipolleschi

Differential Revision: D43040295

Pulled By: cortinico

fbshipit-source-id: e0e5b8fb7faa8ee5654d4cde5f274bef4b517376
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
the current jsc-android is still built based on ndk r21, and react-native is now built based on ndk r23. the unwinder between r21 and r23 is incompatible (libgcc vs libunwind). if there's exceptions throwing from jsc, other react native libraries cannot catch these exceptions and cause runtime crash.

this pr updates jsc-android to 235231.0.0 which is the same webkitgtk version as 235230.2.1 but only built by ndk r23. the jsc-android pr is from react-native-community/jsc-android-buildscripts#179. note that the jsc is based on ndk r23c and react-native is based on ndk r23b. the reason is that i cannot get jsc building successfully on r23b. hopefully r23b and r23c are abi safe.

there is another crash from libjscexecutor when testing the new jsc-android. to fix the issue, i have to explicitly link libunwind.a from libjscexecutor.so. supposedly ndk r23 should help to link libunwind under the hood, i still not figure out why it doesn't. but after linking libunwind.a, i can get new jsc-android work successfully.

```
E/art     ( 2669): dlopen("/data/app/com.test-1/lib/x86_64/libjscexecutor.so", RTLD_LAZY) failed: dlopen failed: cannot locate symbol "_Unwind_Resume" referenced by "/data/app/com.test-1/lib/x86_64/libjscexecutor.so"...
W/System.err( 2669): java.lang.UnsatisfiedLinkError: dlopen failed: cannot locate symbol "_Unwind_Resume" referenced by "/data/app/com.test-1/lib/x86_64/libjscexecutor.so"...
W/System.err( 2669): 	at java.lang.Runtime.load(Runtime.java:331)
W/System.err( 2669): 	at java.lang.System.load(System.java:982)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader$1.load(SoLoader.java:558)
W/System.err( 2669): 	at com.facebook.soloader.DirectorySoSource.loadLibraryFrom(DirectorySoSource.java:110)
W/System.err( 2669): 	at com.facebook.soloader.DirectorySoSource.loadLibrary(DirectorySoSource.java:63)
W/System.err( 2669): 	at com.facebook.soloader.ApplicationSoSource.loadLibrary(ApplicationSoSource.java:91)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.doLoadLibraryBySoName(SoLoader.java:1067)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.loadLibraryBySoNameImpl(SoLoader.java:943)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.loadLibraryBySoName(SoLoader.java:855)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.loadLibrary(SoLoader.java:802)
W/System.err( 2669): 	at com.facebook.soloader.SoLoader.loadLibrary(SoLoader.java:772)
W/System.err( 2669): 	at com.facebook.react.jscexecutor.JSCExecutor.loadLibrary(JSCExecutor.java:24)
W/System.err( 2669): 	at com.facebook.react.jscexecutor.JSCExecutor.<clinit>(JSCExecutor.java:20)
W/System.err( 2669): 	at com.facebook.react.ReactInstanceManagerBuilder.getDefaultJSExecutorFactory(ReactInstanceManagerBuilder.java:363)
W/System.err( 2669): 	at com.facebook.react.ReactInstanceManagerBuilder.build(ReactInstanceManagerBuilder.java:316)
W/System.err( 2669): 	at com.facebook.react.ReactNativeHost.createReactInstanceManager(ReactNativeHost.java:94)
W/System.err( 2669): 	at com.facebook.react.ReactNativeHost.getReactInstanceManager(ReactNativeHost.java:41)
W/System.err( 2669): 	at com.test.MainApplication.onCreate(MainApplication.java:60)
W/System.err( 2669): 	at android.app.Instrumentation.callApplicationOnCreate(Instrumentation.java:1011)
W/System.err( 2669): 	at androidx.test.runner.MonitoringInstrumentation.callApplicationOnCreate(MonitoringInstrumentation.java:483)
W/System.err( 2669): 	at android.app.ActivityThread.handleBindApplication(ActivityThread.java:4518)
W/System.err( 2669): 	at android.app.ActivityThread.access$1500(ActivityThread.java:144)
W/System.err( 2669): 	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1339)
W/System.err( 2669): 	at android.os.Handler.dispatchMessage(Handler.java:102)
W/System.err( 2669): 	at android.os.Looper.loop(Looper.java:135)
W/System.err( 2669): 	at android.app.ActivityThread.main(ActivityThread.java:5221)
W/System.err( 2669): 	at java.lang.reflect.Method.invoke(Native Method)
W/System.err( 2669): 	at java.lang.reflect.Method.invoke(Method.java:372)
W/System.err( 2669): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:899)
W/System.err( 2669): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:694)
```

fixes facebook#36052

## Changelog

[ANDROID][FIXED] - Fixed jscexecutor crash on Android which is caused from NDK incompatibility

Pull Request resolved: facebook#36062

Test Plan: tested on [jsc-android instrumented test](https://github.com/react-native-community/jsc-android-buildscripts/tree/2.26.1/test) (based on react-native 0.71.2)

Reviewed By: cipolleschi

Differential Revision: D43040295

Pulled By: cortinico

fbshipit-source-id: e0e5b8fb7faa8ee5654d4cde5f274bef4b517376
@cipolleschi cipolleschi mentioned this pull request Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. Merged This PR has been merged. p: Expo Partner: Expo Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Current jsc-android prebuilt mismatch NDK version with template app
4 participants