Skip to content

Commit

Permalink
Fix Android native debug build assert crash (facebook#25263)
Browse files Browse the repository at this point in the history
Summary:
The problem happens only from native debug build, i.e.
`NATIVE_BUILD_TYPE=Debug ./gradlew ...`
During Java exception happens, [only limited methods are allowed](https://developer.android.com/training/articles/perf-jni#exceptions_1).
RN uses smart pointer to manage JNI reference.
There is an assert() in smart pointer constructor that will call other JNI method and lead to VM crash.
Since assert() do things in debug build by default, the problem will not happens on native release build.

### Crash backtrace
Java exception happens:

```
java_vm_ext.cc:534] JNI DETECTED ERROR IN APPLICATION: JNI GetObjectRefType called with pending exception com.facebook.react.uimanager.IllegalViewOperationException: No ViewManager defined for class Text
java_vm_ext.cc:534]   at com.facebook.react.uimanager.ViewManager com.facebook.react.uimanager.ViewManagerRegistry.get(java.lang.String) (ViewManagerRegistry.java:57)
java_vm_ext.cc:534]   at com.facebook.react.uimanager.ViewManager com.facebook.react.uimanager.UIImplementation.resolveViewManager(java.lang.String) (UIImplementation.java:134)
java_vm_ext.cc:534]   at com.facebook.react.bridge.WritableMap com.facebook.react.uimanager.UIManagerModule.computeConstantsForViewManager(java.lang.String) (UIManagerModule.java:325)
java_vm_ext.cc:534]   at com.facebook.react.bridge.WritableMap com.facebook.react.uimanager.UIManagerModule.getConstantsForViewManager(java.lang.String) (UIManagerModule.java:319)
java_vm_ext.cc:534]   at void com.facebook.react.bridge.queue.NativeRunnable.run() (NativeRunnable.java:-2)
java_vm_ext.cc:534]   at void android.os.Handler.handleCallback(android.os.Message) (Handler.java:790)
java_vm_ext.cc:534]   at void android.os.Handler.dispatchMessage(android.os.Message) (Handler.java:99)
java_vm_ext.cc:534]   at void com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(android.os.Message) (MessageQueueThreadHandler.java:29)
java_vm_ext.cc:534]   at void android.os.Looper.loop() (Looper.java:164)
java_vm_ext.cc:534]   at void com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run() (MessageQueueThreadImpl.java:232)
java_vm_ext.cc:534]   at void java.lang.Thread.run() (Thread.java:764)
```

Native crash backtrace:

```
#00 pc 00000000000276f8  /system/lib64/libc.so (syscall+24)
facebook#1 pc 0000000000027905  /system/lib64/libc.so (abort+101)
facebook#2 pc 000000000052f32e  /system/lib64/libart.so (art::Runtime::Abort(char const*)+558)
facebook#3 pc 00000000006341a8  /system/lib64/libart.so (android::base::LogMessage::~LogMessage()+1016)
facebook#4 pc 0000000000399339  /system/lib64/libart.so (art::JavaVMExt::JniAbort(char const*, char const*)+1657)
facebook#5 pc 00000000003994c2  /system/lib64/libart.so (art::JavaVMExt::JniAbortV(char const*, char const*, __va_list_tag*)+82)
facebook#6 pc 0000000000178aab  /system/lib64/libart.so (art::ScopedCheck::AbortF(char const*, ...)+187)
facebook#7 pc 00000000001785b8  /system/lib64/libart.so (art::ScopedCheck::CheckThread(_JNIEnv*)+472)
facebook#8 pc 0000000000176871  /system/lib64/libart.so (art::ScopedCheck::Check(art::ScopedObjectAccess&, bool, char const*, art::JniValueType*)+801)
facebook#9 pc 000000000017614a  /system/lib64/libart.so (art::CheckJNI::GetObjectRefType(_JNIEnv*, _jobject*)+778)
facebook#10 pc 0000000000123831  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libreactnativejni.so (_JNIEnv::GetObjectRefType(_jobject*)+49)
facebook#11 pc 00000000001236b1  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libreactnativejni.so (facebook::jni::LocalReferenceAllocator::verifyReference(_jobject*) const+81)
facebook#12 pc 0000000000046f40  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libfb.so (facebook::jni::base_owned_ref<_jthrowable*, facebook::jni::LocalReferenceAllocator>::base_owned_ref(_jthrowable*)+64)
facebook#13 pc 0000000000046ef7  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libfb.so (facebook::jni::basic_strong_ref<_jthrowable*, facebook::jni::LocalReferenceAllocator>::basic_strong_ref(_jthrowable*)+39)
facebook#14 pc 000000000003648b  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libfb.so (_ZN8facebook3jni11adopt_localIP11_jthrowableEENS0_16basic_strong_refIT_NS0_23LocalReferenceAllocatorEEES5_+27)
facebook#15 pc 0000000000036333  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libfb.so (facebook::jni::throwPendingJniExceptionAsCppException()+99)
facebook#16 pc 000000000018652f  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libreactnativejni.so (facebook::react::MethodInvoker::invoke(std::__ndk1::weak_ptr<facebook::react::Instance>&, facebook::jni::alias_ref<facebook::jni::detail::JTypeFor<facebook::react::JBaseJavaModule, facebook::jni::JObject, void>::_javaobject*>, folly::dynamic const&)+4559)
facebook#17 pc 00000000001616fa  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libreactnativejni.so (_ZN8facebook5react16JavaNativeModule26callSerializableNativeHookEjON5folly7dynamicE+954)
facebook#18 pc 000000000021bbd6  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libreactnativejni.so (_ZN8facebook5react14ModuleRegistry26callSerializableNativeHookEjjON5folly7dynamicE+486)
facebook#19 pc 000000000022d90b  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libreactnativejni.so (_ZN8facebook5react16JsToNativeBridge26callSerializableNativeHookERNS0_10JSExecutorEjjON5folly7dynamicE+75)
facebook#20 pc 000000000004a6fa  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libjscexecutor.so (facebook::react::JSIExecutor::nativeCallSyncHook(facebook::jsi::Value const*, unsigned long)+202)
facebook#21 pc 000000000004b7a2  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libjscexecutor.so
facebook#22 pc 0000000000056475  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libjscexecutor.so
facebook#23 pc 00000000000c3a0a  /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libjsc.so
```

Java exception code from [ViewManagerRegistry.java](https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagerRegistry.java#L45-L58) :

```java
  public ViewManager get(String className) {
    ViewManager viewManager = mViewManagers.get(className);
    if (viewManager != null) {
      return viewManager;
    }
    if (mViewManagerResolver != null) {
      viewManager = mViewManagerResolver.getViewManager(className);
      if (viewManager != null) {
        mViewManagers.put(className, viewManager);
        return viewManager;
      }
    }
    throw new IllegalViewOperationException("No ViewManager defined for class " + className);
  }
```
It is an expected exception as [legacy JS try to find view manager](https://github.com/facebook/react-native/blob/master/Libraries/ReactNative/PaperUIManager.js#L44-L50) and Text has no ViewManager registered in fact.

From C++ assert:
`adopt_local`  will call to base_owned_ref() and [there is an assert to ensure JNI reference type](https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/References-inl.h#L203-L209).

```C++
template<typename T, typename Alloc>
inline facebook::jni::base_owned_ref<T, Alloc>::base_owned_ref(
    javaobject reference) noexcept
  : storage_(reference) {
  assert(Alloc{}.verifyReference(reference));
  internal::dbglog("New wrapped ref=%p this=%p", get(), this);
}
```

In the verifyReference(), [there is a GetObjectRefType() call which is an invalid call at exception pending state](https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/ReferenceAllocators-inl.h#L57-L62).

```C++
inline bool LocalReferenceAllocator::verifyReference(jobject reference) const noexcept {
  if (!reference || !internal::doesGetObjectRefTypeWork()) {
    return true;
  }
  return internal::getEnv()->GetObjectRefType(reference) == JNILocalRefType;
}
```

The fix moves `adopt_local()` wrapping after `ExceptionClear()` and we could call `GetObjectRefType()` without problems.

## Changelog

[Android] [Fixed] - Fix Android native debug build assert crash
NOTE this might be ignored in RN release changelog.
Since the problem should exist for years and no user tried native debug build.
Pull Request resolved: facebook#25263

Test Plan:
Try to run RNTester from native debug build and make sure no crash happens.
(It is a launch crash and should be easy to verify)
`NATIVE_BUILD_TYPE=Debug ./gradlew clean :RNTester:android:app:installDebug`

Differential Revision: D15873691

Pulled By: cpojer

fbshipit-source-id: fb93b85daa1136cdf44e7fd7f217a2767391d8dd
  • Loading branch information
Kudo authored and M-i-k-e-l committed Mar 10, 2020
1 parent 9ae0e7c commit 07a0811
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions ReactAndroid/src/main/jni/first-party/fb/jni/Exceptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,13 @@ void throwPendingJniExceptionAsCppException() {
return;
}

auto throwable = adopt_local(env->ExceptionOccurred());
auto throwable = env->ExceptionOccurred();
if (!throwable) {
throw std::runtime_error("Unable to get pending JNI exception.");
}
env->ExceptionClear();

throw JniException(throwable);
throw JniException(adopt_local(throwable));
}

void throwCppExceptionIf(bool condition) {
Expand Down

0 comments on commit 07a0811

Please sign in to comment.