Skip to content

Commit

Permalink
update jsc-android to ndk r23 based (#36062)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Kudo authored and facebook-github-bot committed Feb 7, 2023
1 parent 7e02b41 commit a232dec
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 5 deletions.
13 changes: 13 additions & 0 deletions ReactAndroid/src/main/jni/react/jscexecutor/CMakeLists.txt
Expand Up @@ -13,9 +13,22 @@ add_library(jscexecutor SHARED ${jscexecutor_SRC})

target_include_directories(jscexecutor PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})

# Patch from Expo: https://github.com/expo/react-native/blob/02714ab44d1e206fa80e81aef618e61017cccdc1/ReactAndroid/src/main/java/com/facebook/react/jscexecutor/CMakeLists.txt#L16-L25
# Explicitly link libgcc.a to prevent undefined `_Unwind_Resume` symbol and crash from throwing c++ exceptions even someone tries to catch the exceptions.
# according to https://android.googlesource.com/platform/ndk/+/master/docs/BuildSystemMaintainers.md#unwinding,
# we should put the unwinder between static libs and shared libs.
#
# TODO(ncor): we don't need this patch anymore after upgrading to ndk r23
if(ANDROID_NDK_REVISION VERSION_LESS "23.0.0")
set(LIB_UNWIND gcc)
else()
set(LIB_UNWIND unwind)
endif()

target_link_libraries(jscexecutor
jsireact
jscruntime
${LIB_UNWIND}
fb
fbjni
folly_runtime
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -126,7 +126,7 @@
"event-target-shim": "^5.0.1",
"invariant": "^2.2.4",
"jest-environment-node": "^29.2.1",
"jsc-android": "^250230.2.1",
"jsc-android": "^250231.0.0",
"memoize-one": "^5.0.0",
"metro-react-native-babel-transformer": "0.73.5",
"metro-runtime": "0.73.5",
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Expand Up @@ -5732,10 +5732,10 @@ jsbn@~0.1.0:
resolved "https://registry.yarnpkg.com/jsbn/-/jsbn-0.1.1.tgz#a5e654c2e5a2deb5f201d96cefbca80c0ef2f513"
integrity sha1-peZUwuWi3rXyAdls77yoDA7y9RM=

jsc-android@^250230.2.1:
version "250230.2.1"
resolved "https://registry.yarnpkg.com/jsc-android/-/jsc-android-250230.2.1.tgz#3790313a970586a03ab0ad47defbc84df54f1b83"
integrity sha512-KmxeBlRjwoqCnBBKGsihFtvsBHyUFlBxJPK4FzeYcIuBfdjv6jFys44JITAgSTbQD+vIdwMEfyZklsuQX0yI1Q==
jsc-android@^250231.0.0:
version "250231.0.0"
resolved "https://registry.yarnpkg.com/jsc-android/-/jsc-android-250231.0.0.tgz#91720f8df382a108872fa4b3f558f33ba5e95262"
integrity sha512-rS46PvsjYmdmuz1OAWXY/1kCYG7pnf1TBqeTiOJr1iDz7s5DLxxC9n/ZMknLDxzYzNVfI7R95MH10emSSG1Wuw==

jscodeshift@^0.14.0:
version "0.14.0"
Expand Down

1 comment on commit a232dec

@toscalivia83
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @Kudo
I'm having this exact issue in my repo. Using:
"react-native": "0.71.7",
"jsc-android": "^250231.0.0",

and testing on Android 10. This works fine on other android versions though

Could you help me fix it please?

Please sign in to comment.