Skip to content

Commit

Permalink
Load jsc or hermes lib in static method (#30749)
Browse files Browse the repository at this point in the history
Summary:
Many have reported about the misguiding error `Fatal Exception: java.lang.UnsatisfiedLinkError: couldn't find DSO to load: libhermes.so` even though they don't use Hermes (for example issues #26075 #25923).

**The current code does not handle errors correctly when loading JSC or Hermes in `ReactInstanceManagerBuilder`**.

**ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerBuilder.java:**
```java
try {
  return new HermesExecutorFactory();
} catch (UnsatisfiedLinkError hermesE) {
  // We never get here because "new HermesExecutorFactory()" does not throw an exception!
  hermesE.printStackTrace();
  throw jscE;
}
```

In Java, when an exception is thrown in static block, it will be RuntimeException and it can't be caught. For example the exception from `SoLoader.loadLibrary` can't be caught and it will crash the app.

**ReactAndroid/src/main/java/com/facebook/hermes/reactexecutor/HermesExecutor.java:**
```java
static {
  // Exception from this code block will be RuntimeException and it can't be caught!
  SoLoader.loadLibrary("hermes");
  try {
    SoLoader.loadLibrary("hermes-executor-debug");
    mode_ = "Debug";
  } catch (UnsatisfiedLinkError e) {
    SoLoader.loadLibrary("hermes-executor-release");
    mode_ = "Release";
  }
}
```

This PR fixes the code so that the original exception from failed JSC loading is not swallowed. It does not fix the original issue why JSC loading is failing with some devices, but it can be really helpful to know what the real error is. For example Firebase Crashlytics shows wrong stack trace with current code.

I'm sure that this fix could have been written better. It feels wrong to import `JSCExecutor` and `HermesExecutor` in `ReactInstanceManagerBuilder.java`. However, the main point of this PR is to give the idea what is wrong with the current code.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Android] [Fixed] - Fix error handling when loading JSC or Hermes

Pull Request resolved: #30749

Test Plan:
* from this PR, modify  `ReactAndroid/src/main/java/com/facebook/react/jscexecutor/JSCExecutor.java` so that JSC loading will fail:
```java
// original
SoLoader.loadLibrary("jscexecutor");
// changed
SoLoader.loadLibrary("jscexecutor-does-not-exist");
```
* Run `rn-tester` app
* Check from Logcat that the app crashed with correct exception and stacktrace. It should **not** be `java.lang.UnsatisfiedLinkError: couldn't find DSO to load: libhermes.so`

Tested with Hermes

```
    SoLoader.loadLibrary("hermes-executor-test");
```
Got this one in logcat
```
09-24 20:12:39.552  6412  6455 E AndroidRuntime: java.lang.UnsatisfiedLinkError: couldn't find DSO to load: libhermes-executor-test.so
```

Reviewed By: cortinico

Differential Revision: D30346032

Pulled By: sota000

fbshipit-source-id: 09b032a9e471af233b7ac90b571c311952ab6342
  • Loading branch information
iqqmuT authored and facebook-github-bot committed Oct 21, 2021
1 parent 6acb18c commit d839b24
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,20 @@ public class HermesExecutor extends JavaScriptExecutor {
private static String mode_;

static {
// libhermes must be loaded explicitly to invoke its JNI_OnLoad.
SoLoader.loadLibrary("hermes");
try {
SoLoader.loadLibrary("hermes-executor-debug");
mode_ = "Debug";
} catch (UnsatisfiedLinkError e) {
SoLoader.loadLibrary("hermes-executor-release");
mode_ = "Release";
loadLibrary();
}

public static void loadLibrary() throws UnsatisfiedLinkError {
if (mode_ == null) {
// libhermes must be loaded explicitly to invoke its JNI_OnLoad.
SoLoader.loadLibrary("hermes");
try {
SoLoader.loadLibrary("hermes-executor-debug");
mode_ = "Debug";
} catch (UnsatisfiedLinkError e) {
SoLoader.loadLibrary("hermes-executor-release");
mode_ = "Release";
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import android.app.Application;
import android.content.Context;
import androidx.annotation.Nullable;
import com.facebook.hermes.reactexecutor.HermesExecutor;
import com.facebook.hermes.reactexecutor.HermesExecutorFactory;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.JSBundleLoader;
Expand All @@ -28,6 +29,7 @@
import com.facebook.react.devsupport.RedBoxHandler;
import com.facebook.react.devsupport.interfaces.DevBundleDownloadListener;
import com.facebook.react.devsupport.interfaces.DevSupportManager;
import com.facebook.react.jscexecutor.JSCExecutor;
import com.facebook.react.jscexecutor.JSCExecutorFactory;
import com.facebook.react.modules.core.DefaultHardwareBackBtnHandler;
import com.facebook.react.packagerconnection.RequestHandler;
Expand Down Expand Up @@ -347,7 +349,7 @@ private JavaScriptExecutorFactory getDefaultJSExecutorFactory(
try {
// If JSC is included, use it as normal
initializeSoLoaderIfNecessary(applicationContext);
SoLoader.loadLibrary("jscexecutor");
JSCExecutor.loadLibrary();
return new JSCExecutorFactory(appName, deviceName);
} catch (UnsatisfiedLinkError jscE) {
// https://github.com/facebook/hermes/issues/78 shows that
Expand All @@ -365,6 +367,7 @@ private JavaScriptExecutorFactory getDefaultJSExecutorFactory(

// Otherwise use Hermes
try {
HermesExecutor.loadLibrary();
return new HermesExecutorFactory();
} catch (UnsatisfiedLinkError hermesE) {
// If we get here, either this is a JSC build, and of course
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,13 @@
import com.facebook.soloader.SoLoader;

@DoNotStrip
/* package */ class JSCExecutor extends JavaScriptExecutor {
/* package */ public class JSCExecutor extends JavaScriptExecutor {

static {
loadLibrary();
}

public static void loadLibrary() throws UnsatisfiedLinkError {
SoLoader.loadLibrary("jscexecutor");
}

Expand Down

0 comments on commit d839b24

Please sign in to comment.