From ce80a670bd0f93ec42da9ccdabba3a7e10395815 Mon Sep 17 00:00:00 2001 From: Kudo Chien Date: Tue, 18 Jun 2019 01:57:14 -0700 Subject: [PATCH] Fix Android native debug build assert crash (#25263) 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) https://github.com/facebook/react-native/issues/01 pc 0000000000027905 /system/lib64/libc.so (abort+101) https://github.com/facebook/react-native/issues/02 pc 000000000052f32e /system/lib64/libart.so (art::Runtime::Abort(char const*)+558) https://github.com/facebook/react-native/issues/03 pc 00000000006341a8 /system/lib64/libart.so (android::base::LogMessage::~LogMessage()+1016) https://github.com/facebook/react-native/issues/04 pc 0000000000399339 /system/lib64/libart.so (art::JavaVMExt::JniAbort(char const*, char const*)+1657) https://github.com/facebook/react-native/issues/05 pc 00000000003994c2 /system/lib64/libart.so (art::JavaVMExt::JniAbortV(char const*, char const*, __va_list_tag*)+82) https://github.com/facebook/react-native/issues/06 pc 0000000000178aab /system/lib64/libart.so (art::ScopedCheck::AbortF(char const*, ...)+187) https://github.com/facebook/react-native/issues/07 pc 00000000001785b8 /system/lib64/libart.so (art::ScopedCheck::CheckThread(_JNIEnv*)+472) https://github.com/facebook/react-native/issues/08 pc 0000000000176871 /system/lib64/libart.so (art::ScopedCheck::Check(art::ScopedObjectAccess&, bool, char const*, art::JniValueType*)+801) https://github.com/facebook/react-native/issues/09 pc 000000000017614a /system/lib64/libart.so (art::CheckJNI::GetObjectRefType(_JNIEnv*, _jobject*)+778) https://github.com/facebook/react-native/issues/10 pc 0000000000123831 /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libreactnativejni.so (_JNIEnv::GetObjectRefType(_jobject*)+49) https://github.com/facebook/react-native/issues/11 pc 00000000001236b1 /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libreactnativejni.so (facebook::jni::LocalReferenceAllocator::verifyReference(_jobject*) const+81) https://github.com/facebook/react-native/issues/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) https://github.com/facebook/react-native/issues/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) https://github.com/facebook/react-native/issues/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) https://github.com/facebook/react-native/issues/15 pc 0000000000036333 /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libfb.so (facebook::jni::throwPendingJniExceptionAsCppException()+99) https://github.com/facebook/react-native/issues/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::jni::alias_ref::_javaobject*>, folly::dynamic const&)+4559) https://github.com/facebook/react-native/issues/17 pc 00000000001616fa /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libreactnativejni.so (_ZN8facebook5react16JavaNativeModule26callSerializableNativeHookEjON5folly7dynamicE+954) https://github.com/facebook/react-native/issues/18 pc 000000000021bbd6 /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libreactnativejni.so (_ZN8facebook5react14ModuleRegistry26callSerializableNativeHookEjjON5folly7dynamicE+486) https://github.com/facebook/react-native/issues/19 pc 000000000022d90b /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libreactnativejni.so (_ZN8facebook5react16JsToNativeBridge26callSerializableNativeHookERNS0_10JSExecutorEjjON5folly7dynamicE+75) https://github.com/facebook/react-native/issues/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) https://github.com/facebook/react-native/issues/21 pc 000000000004b7a2 /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libjscexecutor.so https://github.com/facebook/react-native/issues/22 pc 0000000000056475 /data/app/com.facebook.react.uiapp-NEO_tjqQpFriZwGleZxJeQ==/lib/x86_64/libjscexecutor.so https://github.com/facebook/react-native/issues/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 inline facebook::jni::base_owned_ref::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: https://github.com/facebook/react-native/pull/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 --- ReactAndroid/src/main/jni/first-party/fb/jni/Exceptions.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ReactAndroid/src/main/jni/first-party/fb/jni/Exceptions.cpp b/ReactAndroid/src/main/jni/first-party/fb/jni/Exceptions.cpp index 326e114b20de7f..da17efa00cabda 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/jni/Exceptions.cpp +++ b/ReactAndroid/src/main/jni/first-party/fb/jni/Exceptions.cpp @@ -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) {