Use prefab JNI in Hermes

Summary: Instead of including JNI source code in our repo and statically linking in parts of JNI, use the prebuilt shared library available from Maven. While working on fixing the `Intl.NumberFormat` crash in the GC, I got multiple weird exceptions with stack traces that didn't line up with `hermesLog` statements I had added to the code. I eventually suspected something was wrong with having multiple copies of JNI, and switching to the shared library from maven resolved the crashes. Reviewed By: tmikov Differential Revision: D28791954 fbshipit-source-id: 992bc04428795cc3154a229211c1a6f24add32d6
facebook · Jun 8, 2021 · 103f1d3 · 103f1d3
1 parent 1336444
commit 103f1d3
Show file tree

Hide file tree

Showing 10 changed files with 25 additions and 92 deletions.
diff --git a/API/hermes/CMakeLists.txt b/API/hermes/CMakeLists.txt
@@ -11,12 +11,6 @@ set(api_sources
 file(GLOB api_headers ${CMAKE_CURRENT_SOURCE_DIR}/*.h)
 file(GLOB api_public_headers ${PROJECT_SOURCE_DIR}/public/hermes/Public/*.h)
 
-if(HERMES_IS_ANDROID)
-  list(APPEND api_sources
-    JNI_OnLoad.cpp
-    )
-endif()
-
 add_hermes_library(hermesapi
         ${api_sources}
         LINK_LIBS jsi hermesVMRuntime)

diff --git a/API/hermes/JNI_OnLoad.cpp b/API/hermes/JNI_OnLoad.cpp
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -587,20 +587,7 @@ include_directories(BEFORE
   )
 
 if(HERMES_IS_ANDROID)
-  if(EXISTS ${HERMES_SOURCE_DIR}/first-party/fbjni/cxx)
-    set(FBJNI_PATH ${HERMES_SOURCE_DIR}/first-party/fbjni)
-  elseif(EXISTS ${FBSOURCE_DIR}/fbandroid/libraries/fbjni/cxx)
-    set(FBJNI_PATH ${FBSOURCE_DIR}/fbandroid/libraries/fbjni)
-  elseif(EXISTS ${HERMES_SOURCE_DIR}/../../fbandroid/libraries/fbjni/cxx)
-    set(FBJNI_PATH ${HERMES_SOURCE_DIR}/../../fbandroid/libraries/fbjni)
-  else()
-    message(FATAL_ERROR "Unable to find fbjni.")
-  endif()
-  include_directories("${FBJNI_PATH}/cxx/")
-  add_subdirectory(first-party/fbjni)
-
-  # JNI requires that JNI_OnLoad is (re-)exported for initialization.
-  set(OPTIONAL_JNI_ONLOAD "-Wl,--undefined=JNI_OnLoad")
+  find_package(fbjni REQUIRED CONFIG)
 endif()
 
 set(CMAKE_CXX_STANDARD 14)

diff --git a/android/hermes/build.gradle b/android/hermes/build.gradle
@@ -83,15 +83,21 @@ android {
     }
   }
 
+  // Allow using prefab so that we can import libfbjni.so.
+  buildFeatures {
+    prefab true
+  }
+
   dependencies {
-    intlImplementation 'com.facebook.fbjni:fbjni:0.0.2'
+    implementation 'com.facebook.fbjni:fbjni:0.2.2'
     intlImplementation 'com.facebook.soloader:soloader:0.9.0'
     intlImplementation 'com.facebook.yoga:proguard-annotations:1.14.1'
     intlImplementation "androidx.annotation:annotation:1.1.0"
   }
 
   packagingOptions {
     exclude "**/libc++_shared.so"
+    exclude "**/libfbjni.so"
   }
 
   afterEvaluate {

diff --git a/android/intltest/build.gradle b/android/intltest/build.gradle
@@ -86,21 +86,33 @@ android {
 
   useLibrary 'android.test.base'
 
+  // Allow using prefab so that we can import libfbjni.so.
+  buildFeatures {
+    prefab true
+  }
+
   dependencies {
     androidTestImplementation 'junit:junit:4.12'
     androidTestImplementation 'androidx.test.ext:junit:1.1.1'
     androidTestImplementation 'androidx.test.espresso:espresso-core:3.2.0'
     androidTestImplementation 'org.easytesting:fest-assert-core:2.0M10'
-    androidTestImplementation 'com.facebook.fbjni:fbjni:0.0.2'
     androidTestImplementation 'com.facebook.soloader:soloader:0.9.0'
     androidTestImplementation 'com.facebook.yoga:proguard-annotations:1.14.1'
   }
 
   dependencies {
+    implementation 'com.facebook.fbjni:fbjni:0.2.2'
     implementation "androidx.annotation:annotation:1.1.0"
     implementation "androidx.annotation:annotation-experimental:1.0.0"
   }
 
+  // TODO: Revisit this if there is a better solution for deduplicating native
+  // libraries.
+  packagingOptions {
+    pickFirst "**/libfbjni.so"
+    pickFirst "**/libc++_shared.so"
+  }
+
   sourceSets {
     main {
       java {

diff --git a/android/intltest/java/com/facebook/hermes/test/CMakeLists.txt b/android/intltest/java/com/facebook/hermes/test/CMakeLists.txt
@@ -30,7 +30,7 @@ if(HERMES_IS_ANDROID)
       libhermes
       hermesapi
       compileJS
-      hermesfbjni
+      fbjni::fbjni
       jsi
       ${CORE_FOUNDATION}
   )
@@ -43,7 +43,7 @@ if(HERMES_IS_ANDROID)
       libhermes
       hermesapi
       compileJS
-      hermesfbjni
+      fbjni::fbjni
       jsi
       ${CORE_FOUNDATION}
   )

diff --git a/first-party/README b/first-party/README
diff --git a/first-party/fbjni/CMakeLists.txt b/first-party/fbjni/CMakeLists.txt
diff --git a/lib/Platform/Intl/CMakeLists.txt b/lib/Platform/Intl/CMakeLists.txt
@@ -11,7 +11,7 @@ set(source_files
 if(HERMES_IS_ANDROID)
   add_hermes_library(hermesPlatformIntl STATIC ${source_files}
       LINK_LIBS
-      hermesfbjni
+      fbjni::fbjni
       ${CORE_FOUNDATION}
   )
   set_source_files_properties(PlatformIntlAndroid.cpp PROPERTIES

diff --git a/lib/Platform/Unicode/CMakeLists.txt b/lib/Platform/Unicode/CMakeLists.txt
@@ -14,7 +14,7 @@ set(source_files
 if(HERMES_IS_ANDROID)
   add_hermes_library(hermesPlatformUnicode STATIC ${source_files}
       LINK_LIBS
-      hermesfbjni
+      fbjni::fbjni
       ${CORE_FOUNDATION}
   )
   set_source_files_properties(PlatformUnicodeJava.cpp PROPERTIES