From 4978de43bae985042fce5d45df88553eba5d69ef Mon Sep 17 00:00:00 2001 From: Konstantin Raev Date: Sat, 2 Jul 2016 19:54:18 +0100 Subject: [PATCH 1/2] Turned old bridge for OSS back by default This should revert back to using old bridge by default until we fix gradle script to build new bridge for OSS correctly Test Plan: - circle CI - travis - sandcastle --- .../java/com/facebook/react/testing/ReactAppTestActivity.java | 1 + .../src/main/java/com/facebook/react/ReactNativeHost.java | 1 + 2 files changed, 2 insertions(+) diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactAppTestActivity.java b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactAppTestActivity.java index 37f0a4db3955..592324e26db9 100644 --- a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactAppTestActivity.java +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactAppTestActivity.java @@ -153,6 +153,7 @@ public void loadApp( ReactInstanceManager.Builder builder = ReactTestHelper.getReactTestFactory().getReactInstanceManagerBuilder() .setApplication(getApplication()) + .setUseOldBridge(true) .setBundleAssetName(bundleName) // By not setting a JS module name, we force the bundle to be always loaded from // assets, not the devserver, even if dev mode is enabled (such as when testing redboxes). diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactNativeHost.java b/ReactAndroid/src/main/java/com/facebook/react/ReactNativeHost.java index b302024426b3..54e893f1e4e4 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactNativeHost.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactNativeHost.java @@ -61,6 +61,7 @@ public void clear() { protected ReactInstanceManager createReactInstanceManager() { ReactInstanceManager.Builder builder = ReactInstanceManager.builder() + .setUseOldBridge(true) .setApplication(mApplication) .setJSMainModuleName(getJSMainModuleName()) .setUseDeveloperSupport(getUseDeveloperSupport()) From 5f436e35b5eefbd72a2411b96ed2d78928287366 Mon Sep 17 00:00:00 2001 From: Konstantin Raev Date: Sat, 2 Jul 2016 21:14:18 +0100 Subject: [PATCH 2/2] Revert "Pull an updated version of fbjni into RN OSS" This reverts commit 8b2adeaa2e969670b288b54310d527d0e6bae085. --- .../first-party/fb/include/fb/Environment.h | 57 ++- .../first-party/fb/include/fb/fbjni/Common.h | 20 - .../first-party/fb/include/fb/fbjni/Context.h | 4 - .../fb/include/fb/fbjni/CoreClasses-inl.h | 7 - .../fb/include/fb/fbjni/CoreClasses.h | 4 +- .../fb/include/fb/fbjni/Exceptions.h | 54 ++- .../fb/include/fb/fbjni/Meta-inl.h | 28 +- .../fb/include/fb/fbjni/NativeRunnable.h | 43 -- .../fb/fbjni/ReferenceAllocators-inl.h | 2 + .../first-party/fb/include/jni/LocalString.h | 4 +- .../jni/first-party/fb/jni/Environment.cpp | 47 +-- .../jni/first-party/fb/jni/Exceptions.cpp | 384 ++++++++++++------ .../jni/first-party/fb/jni/LocalString.cpp | 4 +- .../main/jni/first-party/fb/jni/OnLoad.cpp | 3 - .../src/main/jni/first-party/fb/jni/fbjni.cpp | 7 +- 15 files changed, 335 insertions(+), 333 deletions(-) delete mode 100644 ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/NativeRunnable.h diff --git a/ReactAndroid/src/main/jni/first-party/fb/include/fb/Environment.h b/ReactAndroid/src/main/jni/first-party/fb/include/fb/Environment.h index 64f9937a6577..bf6c36162afc 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/include/fb/Environment.h +++ b/ReactAndroid/src/main/jni/first-party/fb/include/fb/Environment.h @@ -8,7 +8,6 @@ */ #pragma once -#include #include #include @@ -22,35 +21,44 @@ struct Environment { // May be null if this thread isn't attached to the JVM FBEXPORT static JNIEnv* current(); static void initialize(JavaVM* vm); - - // There are subtle issues with calling the next functions directly. It is - // much better to always use a ThreadScope to manage attaching/detaching for - // you. FBEXPORT static JNIEnv* ensureCurrentThreadIsAttached(); FBEXPORT static void detachCurrentThread(); }; /** - * RAII Object that attaches a thread to the JVM. Failing to detach from a thread before it - * exits will cause a crash, as will calling Detach an extra time, and this guard class helps - * keep that straight. In addition, it remembers whether it performed the attach or not, so it - * is safe to nest it with itself or with non-fbjni code that manages the attachment correctly. + * RAII Object that attaches a thread to the JVM. Failing to detach from a + * thread before it + * exits will cause a crash, as will calling Detach an extra time, and this + * guard class helps + * keep that straight. In addition, it remembers whether it performed the attach + * or not, so it + * is safe to nest it with itself or with non-fbjni code that manages the + * attachment correctly. * * Potential concerns: - * - Attaching to the JVM is fast (~100us on MotoG), but ideally you would attach while the + * - Attaching to the JVM is fast (~100us on MotoG), but ideally you would + * attach while the * app is not busy. - * - Having a thread detach at arbitrary points is not safe in Dalvik; you need to be sure that - * there is no Java code on the current stack or you run the risk of a crash like: + * - Having a thread detach at arbitrary points is not safe in Dalvik; you need + * to be sure that + * there is no Java code on the current stack or you run the risk of a crash + * like: * ERROR: detaching thread with interp frames (count=18) - * (More detail at https://groups.google.com/forum/#!topic/android-ndk/2H8z5grNqjo) - * ThreadScope won't do a detach if the thread was already attached before the guard is + * (More detail at + * https://groups.google.com/forum/#!topic/android-ndk/2H8z5grNqjo) + * ThreadScope won't do a detach if the thread was already attached before + * the guard is * instantiated, but there's probably some usage that could trip this up. - * - Newly attached C++ threads only get the bootstrap class loader -- i.e. java language - * classes, not any of our application's classes. This will be different behavior than threads - * that were initiated on the Java side. A workaround is to pass a global reference for a - * class or instance to the new thread; this bypasses the need for the class loader. - * (See http://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/invocation.html#attach_current_thread) - * If you need access to the application's classes, you can use ThreadScope::WithClassLoader. + * - Newly attached C++ threads only get the bootstrap class loader -- i.e. + * java language + * classes, not any of our application's classes. This will be different + * behavior than threads + * that were initiated on the Java side. A workaround is to pass a global + * reference for a + * class or instance to the new thread; this bypasses the need for the class + * loader. + * (See + * http://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/invocation.html#attach_current_thread) */ class FBEXPORT ThreadScope { public: @@ -61,15 +69,6 @@ class FBEXPORT ThreadScope { ThreadScope& operator=(ThreadScope&&) = delete; ~ThreadScope(); - /** - * This runs the closure in a scope with fbjni's classloader. This should be - * the same classloader as the rest of the application and thus anything - * running in the closure will have access to the same classes as in a normal - * java-create thread. - */ - static void WithClassLoader(std::function&& runnable); - - static void OnLoad(); private: bool attachedWithThisScope_; }; diff --git a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Common.h b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Common.h index 9da51c406e98..5db5293044a6 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Common.h +++ b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Common.h @@ -29,31 +29,11 @@ # endif #endif -// If a pending JNI Java exception is found, wraps it in a JniException object and throws it as -// a C++ exception. -#define FACEBOOK_JNI_THROW_PENDING_EXCEPTION() \ - ::facebook::jni::throwPendingJniExceptionAsCppException() - -// If the condition is true, throws a JniException object, which wraps the pending JNI Java -// exception if any. If no pending exception is found, throws a JniException object that wraps a -// RuntimeException throwable.  -#define FACEBOOK_JNI_THROW_EXCEPTION_IF(CONDITION) \ - ::facebook::jni::throwCppExceptionIf(CONDITION) - /// @cond INTERNAL namespace facebook { namespace jni { -FBEXPORT void throwPendingJniExceptionAsCppException(); -FBEXPORT void throwCppExceptionIf(bool condition); - -[[noreturn]] FBEXPORT void throwNewJavaException(jthrowable); -[[noreturn]] FBEXPORT void throwNewJavaException(const char* throwableName, const char* msg); -template -[[noreturn]] void throwNewJavaException(const char* throwableName, const char* fmt, Args... args); - - /** * This needs to be called at library load time, typically in your JNI_OnLoad method. * diff --git a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Context.h b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Context.h index 8630aa6bf49d..136ca82f1917 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Context.h +++ b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Context.h @@ -25,10 +25,6 @@ class AContext : public JavaClass { return method(self()); } - local_ref getFilesDir() { - static auto method = getClass()->getMethod("getFilesDir"); - return method(self()); - } }; } diff --git a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/CoreClasses-inl.h b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/CoreClasses-inl.h index 40fb94b7b5a0..f5f861ba3c6d 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/CoreClasses-inl.h +++ b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/CoreClasses-inl.h @@ -341,13 +341,6 @@ struct Convert { }; } -// jthrowable ////////////////////////////////////////////////////////////////////////////////////// - -inline local_ref JThrowable::initCause(alias_ref cause) { - static auto meth = javaClassStatic()->getMethod("initCause"); - return meth(self(), cause.get()); -} - // jtypeArray ////////////////////////////////////////////////////////////////////////////////////// namespace detail { diff --git a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/CoreClasses.h b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/CoreClasses.h index 9e0bb87641f1..62b8bb7fe810 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/CoreClasses.h +++ b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/CoreClasses.h @@ -212,7 +212,7 @@ class FBEXPORT JavaClass : public Base { /// the Java class actually has (i.e. with static create() functions). template static local_ref newInstance(Args... args) { - return detail::newInstance(args...); + return detail::newInstance(args...); } javaobject self() const noexcept; @@ -344,8 +344,6 @@ FBEXPORT local_ref make_jstring(const std::string& modifiedUtf8); class FBEXPORT JThrowable : public JavaClass { public: static constexpr const char* kJavaDescriptor = "Ljava/lang/Throwable;"; - - local_ref initCause(alias_ref cause); }; namespace detail { diff --git a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Exceptions.h b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Exceptions.h index ce2cc70bdc3f..8edb871c2d3c 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Exceptions.h +++ b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Exceptions.h @@ -29,26 +29,30 @@ #include #include "Common.h" -#include "References.h" -#include "CoreClasses.h" -namespace facebook { -namespace jni { +// If a pending JNI Java exception is found, wraps it in a JniException object and throws it as +// a C++ exception. +#define FACEBOOK_JNI_THROW_PENDING_EXCEPTION() \ + ::facebook::jni::throwPendingJniExceptionAsCppException() -class JThrowable; +// If the condition is true, throws a JniException object, which wraps the pending JNI Java +// exception if any. If no pending exception is found, throws a JniException object that wraps a +// RuntimeException throwable.  +#define FACEBOOK_JNI_THROW_EXCEPTION_IF(CONDITION) \ + ::facebook::jni::throwCppExceptionIf(CONDITION) -class JCppException : public JavaClass { - public: - static auto constexpr kJavaDescriptor = "Lcom/facebook/jni/CppException;"; +namespace facebook { +namespace jni { - static local_ref create(const char* str) { - return newInstance(make_jstring(str)); - } +namespace internal { + void initExceptionHelpers(); +} - static local_ref create(const std::exception& ex) { - return newInstance(make_jstring(ex.what())); - } -}; +/** + * Before using any of the state initialized above, call this. It + * will assert if initialization has not yet occurred. + */ +FBEXPORT void assertIfExceptionsNotInitialized(); // JniException //////////////////////////////////////////////////////////////////////////////////// @@ -63,22 +67,23 @@ class JCppException : public JavaClass { class FBEXPORT JniException : public std::exception { public: JniException(); - ~JniException(); - explicit JniException(alias_ref throwable); + explicit JniException(jthrowable throwable); JniException(JniException &&rhs); JniException(const JniException &other); - local_ref getThrowable() const noexcept; + ~JniException() noexcept; + + jthrowable getThrowable() const noexcept; virtual const char* what() const noexcept; void setJavaException() const noexcept; private: - global_ref throwable_; + jthrowable throwableGlobalRef_; mutable std::string what_; mutable bool isMessageExtracted_; const static std::string kExceptionMessageFailure_; @@ -90,8 +95,16 @@ class FBEXPORT JniException : public std::exception { // Functions that throw C++ exceptions +FBEXPORT void throwPendingJniExceptionAsCppException(); + +FBEXPORT void throwCppExceptionIf(bool condition); + static const int kMaxExceptionMessageBufferSize = 512; +[[noreturn]] FBEXPORT void throwNewJavaException(jthrowable); + +[[noreturn]] FBEXPORT void throwNewJavaException(const char* throwableName, const char* msg); + // These methods are the preferred way to throw a Java exception from // a C++ function. They create and throw a C++ exception which wraps // a Java exception, so the C++ flow is interrupted. Then, when @@ -100,6 +113,7 @@ static const int kMaxExceptionMessageBufferSize = 512; // thrown to the java caller. template [[noreturn]] void throwNewJavaException(const char* throwableName, const char* fmt, Args... args) { + assertIfExceptionsNotInitialized(); int msgSize = snprintf(nullptr, 0, fmt, args...); char *msg = (char*) alloca(msgSize + 1); @@ -109,7 +123,7 @@ template // Identifies any pending C++ exception and throws it as a Java exception. If the exception can't // be thrown, it aborts the program. This is a noexcept function at C++ level. -FBEXPORT void translatePendingCppExceptionToJavaException() noexcept; +void translatePendingCppExceptionToJavaException() noexcept; // For convenience, some exception names in java.lang are available here. diff --git a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Meta-inl.h b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Meta-inl.h index 45cf2d1bd802..dda35925dbc0 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Meta-inl.h +++ b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/Meta-inl.h @@ -66,7 +66,7 @@ local_ref::javaobject> makeArgsArray(Args... args) { } -inline bool needsSlowPath(alias_ref obj) { +bool needsSlowPath(alias_ref obj) { #if defined(__ANDROID__) // On Android 6.0, art crashes when attempting to call a function on a Proxy. // So, when we detect that case we must use the safe, slow workaround. That is, @@ -88,6 +88,19 @@ inline bool needsSlowPath(alias_ref obj) { } +template +local_ref slowCall(jmethodID method_id, alias_ref self, Args... args) { + static auto invoke = findClassStatic("java/lang/reflect/Method") + ->getMethod::javaobject)>("invoke"); + // TODO(xxxxxxx): Provide fbjni interface to ToReflectedMethod. + auto reflected = adopt_local(Environment::current()->ToReflectedMethod(self->getClass().get(), method_id, JNI_FALSE)); + FACEBOOK_JNI_THROW_PENDING_EXCEPTION(); + if (!reflected) throw JniException(); + auto argsArray = makeArgsArray(args...); + // No need to check for exceptions since invoke is itself a JMethod that will do that for us. + return invoke(reflected, self.get(), argsArray.get()); +} + template inline void JMethod::operator()(alias_ref self, Args... args) { const auto env = Environment::current(); @@ -272,19 +285,6 @@ class JNonvirtualMethod : public JMethodBase { friend class JClass; }; -template -local_ref slowCall(jmethodID method_id, alias_ref self, Args... args) { - static auto invoke = findClassStatic("java/lang/reflect/Method") - ->getMethod::javaobject)>("invoke"); - // TODO(xxxxxxx): Provide fbjni interface to ToReflectedMethod. - auto reflected = adopt_local(Environment::current()->ToReflectedMethod(self->getClass().get(), method_id, JNI_FALSE)); - FACEBOOK_JNI_THROW_PENDING_EXCEPTION(); - if (!reflected) throw std::runtime_error("Unable to get reflected java.lang.reflect.Method"); - auto argsArray = makeArgsArray(args...); - // No need to check for exceptions since invoke is itself a JMethod that will do that for us. - return invoke(reflected, self.get(), argsArray.get()); -} - // JField /////////////////////////////////////////////////////////////////////////////////////// diff --git a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/NativeRunnable.h b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/NativeRunnable.h deleted file mode 100644 index f96f4ee4691e..000000000000 --- a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/NativeRunnable.h +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright (c) 2015-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - */ - -#pragma once - -#include "CoreClasses.h" -#include "Hybrid.h" -#include "Registration.h" - -#include - -namespace facebook { -namespace jni { - -struct JNativeRunnable : public HybridClass { - public: - static auto constexpr kJavaDescriptor = "Lcom/facebook/jni/NativeRunnable;"; - - JNativeRunnable(std::function&& runnable) : runnable_(std::move(runnable)) {} - - static void OnLoad() { - registerHybrid({ - makeNativeMethod("run", JNativeRunnable::run), - }); - } - - void run() { - runnable_(); - } - - private: - std::function runnable_; -}; - - -} // namespace jni -} // namespace facebook diff --git a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/ReferenceAllocators-inl.h b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/ReferenceAllocators-inl.h index 9106dd2532a2..5f69a3b41503 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/ReferenceAllocators-inl.h +++ b/ReactAndroid/src/main/jni/first-party/fb/include/fb/fbjni/ReferenceAllocators-inl.h @@ -13,6 +13,8 @@ #include #include +#include "Exceptions.h" + namespace facebook { namespace jni { diff --git a/ReactAndroid/src/main/jni/first-party/fb/include/jni/LocalString.h b/ReactAndroid/src/main/jni/first-party/fb/include/jni/LocalString.h index 7278fc848f8d..2763e62be8d7 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/include/jni/LocalString.h +++ b/ReactAndroid/src/main/jni/first-party/fb/include/jni/LocalString.h @@ -23,8 +23,8 @@ namespace detail { void utf8ToModifiedUTF8(const uint8_t* bytes, size_t len, uint8_t* modified, size_t modifiedLength); size_t modifiedLength(const std::string& str); size_t modifiedLength(const uint8_t* str, size_t* length); -std::string modifiedUTF8ToUTF8(const uint8_t* modified, size_t len) noexcept; -std::string utf16toUTF8(const uint16_t* utf16Bytes, size_t len) noexcept; +std::string modifiedUTF8ToUTF8(const uint8_t* modified, size_t len); +std::string utf16toUTF8(const uint16_t* utf16Bytes, size_t len); } diff --git a/ReactAndroid/src/main/jni/first-party/fb/jni/Environment.cpp b/ReactAndroid/src/main/jni/first-party/fb/jni/Environment.cpp index e8ea51dc0687..60f63ef6bf72 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/jni/Environment.cpp +++ b/ReactAndroid/src/main/jni/first-party/fb/jni/Environment.cpp @@ -12,40 +12,12 @@ #include #include #include -#include -#include - -#include namespace facebook { namespace jni { -namespace { -StaticInitialized> g_env; -JavaVM* g_vm = nullptr; - -struct JThreadScopeSupport : JavaClass { - static auto constexpr kJavaDescriptor = "Lcom/facebook/jni/ThreadScopeSupport;"; - - // These reinterpret_casts are a totally dangerous pattern. Don't use them. Use HybridData instead. - static void runStdFunction(std::function&& func) { - static auto method = javaClassStatic()->getStaticMethod("runStdFunction"); - method(javaClassStatic(), reinterpret_cast(&func)); - } - - static void runStdFunctionImpl(alias_ref, jlong ptr) { - (*reinterpret_cast*>(ptr))(); - } - - static void OnLoad() { - // We need the javaClassStatic so that the class lookup is cached and that - // runStdFunction can be called from a ThreadScope-attached thread. - javaClassStatic()->registerNatives({ - makeNativeMethod("runStdFunctionImpl", runStdFunctionImpl), - }); - } -}; -} +static StaticInitialized> g_env; +static JavaVM* g_vm = nullptr; /* static */ JNIEnv* Environment::current() { @@ -53,7 +25,6 @@ JNIEnv* Environment::current() { if ((env == nullptr) && (g_vm != nullptr)) { if (g_vm->GetEnv((void**) &env, JNI_VERSION_1_6) != JNI_OK) { FBLOGE("Error retrieving JNI Environment, thread is probably not attached to JVM"); - // TODO(cjhopman): This should throw an exception. env = nullptr; } else { g_env->reset(env); @@ -114,19 +85,5 @@ ThreadScope::~ThreadScope() { } } -/* static */ -void ThreadScope::OnLoad() { - // These classes are required for ScopeWithClassLoader. Ensure they are looked up when loading. - JThreadScopeSupport::OnLoad(); -} - -/* static */ -void ThreadScope::WithClassLoader(std::function&& runnable) { - // TODO(cjhopman): If the classloader is already available in this scope, we - // shouldn't have to jump through java. - ThreadScope ts; - JThreadScopeSupport::runStdFunction(std::move(runnable)); -} - } } 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 bfc99214a904..e7d38054cfd5 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/jni/Exceptions.cpp +++ b/ReactAndroid/src/main/jni/first-party/fb/jni/Exceptions.cpp @@ -7,10 +7,9 @@ * of patent rights can be found in the PATENTS file in the same directory. */ -#include +#include "fb/fbjni.h" #include -#include #include #include @@ -22,113 +21,234 @@ #include - namespace facebook { namespace jni { -namespace { -class JRuntimeException : public JavaClass { +// CommonJniExceptions ///////////////////////////////////////////////////////////////////////////// + +class FBEXPORT CommonJniExceptions { public: - static auto constexpr kJavaDescriptor = "Ljava/lang/RuntimeException;"; + static void init(); - static local_ref create(const char* str) { - return newInstance(make_jstring(str)); + static jclass getThrowableClass() { + return throwableClass_; } - static local_ref create() { - return newInstance(); + static jclass getUnknownCppExceptionClass() { + return unknownCppExceptionClass_; } -}; - -class JIOException : public JavaClass { - public: - static auto constexpr kJavaDescriptor = "Ljava/io/IOException;"; - static local_ref create(const char* str) { - return newInstance(make_jstring(str)); + static jthrowable getUnknownCppExceptionObject() { + return unknownCppExceptionObject_; } -}; - -class JOutOfMemoryError : public JavaClass { - public: - static auto constexpr kJavaDescriptor = "Ljava/lang/OutOfMemoryError;"; - static local_ref create(const char* str) { - return newInstance(make_jstring(str)); + static jthrowable getRuntimeExceptionObject() { + return runtimeExceptionObject_; } -}; - -class JArrayIndexOutOfBoundsException : public JavaClass { - public: - static auto constexpr kJavaDescriptor = "Ljava/lang/ArrayIndexOutOfBoundsException;"; - static local_ref create(const char* str) { - return newInstance(make_jstring(str)); - } + private: + static jclass throwableClass_; + static jclass unknownCppExceptionClass_; + static jthrowable unknownCppExceptionObject_; + static jthrowable runtimeExceptionObject_; }; -class JUnknownCppException : public JavaClass { - public: - static auto constexpr kJavaDescriptor = "Lcom/facebook/jni/UnknownCppException;"; +// The variables in this class are all JNI global references and are intentionally leaked because +// we assume this library cannot be unloaded. These global references are created manually instead +// of using global_ref from References.h to avoid circular dependency. +jclass CommonJniExceptions::throwableClass_ = nullptr; +jclass CommonJniExceptions::unknownCppExceptionClass_ = nullptr; +jthrowable CommonJniExceptions::unknownCppExceptionObject_ = nullptr; +jthrowable CommonJniExceptions::runtimeExceptionObject_ = nullptr; + + +// Variable to guarantee that fallback exceptions have been initialized early. We don't want to +// do pure dynamic initialization -- we want to warn programmers early that they need to run the +// helpers at library load time instead of lazily getting them when the exception helpers are +// first used. +static std::atomic gIsInitialized(false); + +void CommonJniExceptions::init() { + JNIEnv* env = internal::getEnv(); + FBASSERTMSGF(env, "Could not get JNI Environment"); + + // Throwable class + jclass localThrowableClass = env->FindClass("java/lang/Throwable"); + FBASSERT(localThrowableClass); + throwableClass_ = static_cast(env->NewGlobalRef(localThrowableClass)); + FBASSERT(throwableClass_); + env->DeleteLocalRef(localThrowableClass); + + // UnknownCppException class + jclass localUnknownCppExceptionClass = env->FindClass("com/facebook/jni/UnknownCppException"); + FBASSERT(localUnknownCppExceptionClass); + jmethodID unknownCppExceptionConstructorMID = env->GetMethodID( + localUnknownCppExceptionClass, + "", + "()V"); + FBASSERT(unknownCppExceptionConstructorMID); + unknownCppExceptionClass_ = static_cast(env->NewGlobalRef(localUnknownCppExceptionClass)); + FBASSERT(unknownCppExceptionClass_); + env->DeleteLocalRef(localUnknownCppExceptionClass); + + // UnknownCppException object + jthrowable localUnknownCppExceptionObject = static_cast(env->NewObject( + unknownCppExceptionClass_, + unknownCppExceptionConstructorMID)); + FBASSERT(localUnknownCppExceptionObject); + unknownCppExceptionObject_ = static_cast(env->NewGlobalRef( + localUnknownCppExceptionObject)); + FBASSERT(unknownCppExceptionObject_); + env->DeleteLocalRef(localUnknownCppExceptionObject); + + // RuntimeException object + jclass localRuntimeExceptionClass = env->FindClass("java/lang/RuntimeException"); + FBASSERT(localRuntimeExceptionClass); + + jmethodID runtimeExceptionConstructorMID = env->GetMethodID( + localRuntimeExceptionClass, + "", + "()V"); + FBASSERT(runtimeExceptionConstructorMID); + jthrowable localRuntimeExceptionObject = static_cast(env->NewObject( + localRuntimeExceptionClass, + runtimeExceptionConstructorMID)); + FBASSERT(localRuntimeExceptionObject); + runtimeExceptionObject_ = static_cast(env->NewGlobalRef(localRuntimeExceptionObject)); + FBASSERT(runtimeExceptionObject_); + + env->DeleteLocalRef(localRuntimeExceptionClass); + env->DeleteLocalRef(localRuntimeExceptionObject); +} - static local_ref create() { - return newInstance(); - } - static local_ref create(const char* str) { - return newInstance(make_jstring(str)); - } -}; +// initExceptionHelpers() ////////////////////////////////////////////////////////////////////////// -class JCppSystemErrorException : public JavaClass { - public: - static auto constexpr kJavaDescriptor = "Lcom/facebook/jni/CppSystemErrorException;"; +void internal::initExceptionHelpers() { + CommonJniExceptions::init(); + gIsInitialized.store(true, std::memory_order_seq_cst); +} - static local_ref create(const std::system_error& e) { - return newInstance(make_jstring(e.what()), e.code().value()); - } -}; +void assertIfExceptionsNotInitialized() { + // Use relaxed memory order because we don't need memory barriers. + // The real init-once enforcement is done by the compiler for the + // "static" in initExceptionHelpers. + FBASSERTMSGF(gIsInitialized.load(std::memory_order_relaxed), + "initExceptionHelpers was never called!"); +} // Exception throwing & translating functions ////////////////////////////////////////////////////// // Functions that throw Java exceptions -void setJavaExceptionAndAbortOnFailure(alias_ref throwable) { - auto env = Environment::current(); +namespace { + +void setJavaExceptionAndAbortOnFailure(jthrowable throwable) noexcept { + assertIfExceptionsNotInitialized(); + JNIEnv* env = internal::getEnv(); if (throwable) { - env->Throw(throwable.get()); + env->Throw(throwable); } if (env->ExceptionCheck() != JNI_TRUE) { std::abort(); } } +void setDefaultException() noexcept { + assertIfExceptionsNotInitialized(); + setJavaExceptionAndAbortOnFailure(CommonJniExceptions::getRuntimeExceptionObject()); +} + +void setCppSystemErrorExceptionInJava(const std::system_error& ex) noexcept { + assertIfExceptionsNotInitialized(); + JNIEnv* env = internal::getEnv(); + jclass cppSystemErrorExceptionClass = env->FindClass( + "com/facebook/jni/CppSystemErrorException"); + if (!cppSystemErrorExceptionClass) { + setDefaultException(); + return; + } + jmethodID constructorMID = env->GetMethodID( + cppSystemErrorExceptionClass, + "", + "(Ljava/lang/String;I)V"); + if (!constructorMID) { + setDefaultException(); + return; + } + jthrowable cppSystemErrorExceptionObject = static_cast(env->NewObject( + cppSystemErrorExceptionClass, + constructorMID, + env->NewStringUTF(ex.what()), + ex.code().value())); + setJavaExceptionAndAbortOnFailure(cppSystemErrorExceptionObject); +} + +template +void setNewJavaException(jclass exceptionClass, const char* fmt, ARGS... args) { + assertIfExceptionsNotInitialized(); + int msgSize = snprintf(nullptr, 0, fmt, args...); + JNIEnv* env = internal::getEnv(); + + try { + char *msg = (char*) alloca(msgSize + 1); + snprintf(msg, kMaxExceptionMessageBufferSize, fmt, args...); + env->ThrowNew(exceptionClass, msg); + } catch (...) { + env->ThrowNew(exceptionClass, ""); + } + + if (env->ExceptionCheck() != JNI_TRUE) { + setDefaultException(); + } +} + +void setNewJavaException(jclass exceptionClass, const char* msg) { + assertIfExceptionsNotInitialized(); + setNewJavaException(exceptionClass, "%s", msg); +} + +template +void setNewJavaException(const char* className, const char* fmt, ARGS... args) { + assertIfExceptionsNotInitialized(); + JNIEnv* env = internal::getEnv(); + jclass exceptionClass = env->FindClass(className); + if (env->ExceptionCheck() != JNI_TRUE && !exceptionClass) { + // If FindClass() has failed but no exception has been thrown, throw a default exception. + setDefaultException(); + return; + } + setNewJavaException(exceptionClass, fmt, args...); +} + } // Functions that throw C++ exceptions // TODO(T6618159) Take a stack dump here to save context if it results in a crash when propagated -void throwPendingJniExceptionAsCppException() { - JNIEnv* env = Environment::current(); +FBEXPORT void throwPendingJniExceptionAsCppException() { + assertIfExceptionsNotInitialized(); + JNIEnv* env = internal::getEnv(); if (env->ExceptionCheck() == JNI_FALSE) { return; } - auto throwable = adopt_local(env->ExceptionOccurred()); + jthrowable throwable = env->ExceptionOccurred(); if (!throwable) { throw std::runtime_error("Unable to get pending JNI exception."); } - env->ExceptionClear(); + env->ExceptionClear(); throw JniException(throwable); } void throwCppExceptionIf(bool condition) { + assertIfExceptionsNotInitialized(); if (!condition) { return; } - auto env = Environment::current(); + JNIEnv* env = internal::getEnv(); if (env->ExceptionCheck() == JNI_TRUE) { throwPendingJniExceptionAsCppException(); return; @@ -137,11 +257,13 @@ void throwCppExceptionIf(bool condition) { throw JniException(); } -void throwNewJavaException(jthrowable throwable) { - throw JniException(wrap_alias(throwable)); +FBEXPORT void throwNewJavaException(jthrowable throwable) { + throw JniException(throwable); } -void throwNewJavaException(const char* throwableName, const char* msg) { +FBEXPORT void throwNewJavaException( + const char* throwableName, + const char* msg) { // If anything of the fbjni calls fail, an exception of a suitable // form will be thrown, which is what we want. auto throwableClass = findClassLocal(throwableName); @@ -153,80 +275,34 @@ void throwNewJavaException(const char* throwableName, const char* msg) { // Translate C++ to Java Exception -namespace { - -// The implementation std::rethrow_if_nested uses a dynamic_cast to determine -// if the exception is a nested_exception. If the exception is from a library -// built with -fno-rtti, then that will crash. This avoids that. -void rethrow_if_nested() { - try { - throw; - } catch (const std::nested_exception& e) { - e.rethrow_nested(); - } catch (...) { - } -} - -// For each exception in the chain of the currently handled exception, func -// will be called with that exception as the currently handled exception (in -// reverse order, i.e. innermost first). -void denest(std::function func) { +FBEXPORT void translatePendingCppExceptionToJavaException() noexcept { + assertIfExceptionsNotInitialized(); try { - throw; - } catch (const std::exception& e) { - try { - rethrow_if_nested(); - } catch (...) { - denest(func); - } - func(); - } catch (...) { - func(); - } -} -} - -void translatePendingCppExceptionToJavaException() noexcept { - local_ref previous; - auto func = [&previous] () { - local_ref current; try { throw; } catch(const JniException& ex) { - current = ex.getThrowable(); + ex.setJavaException(); } catch(const std::ios_base::failure& ex) { - current = JIOException::create(ex.what()); + setNewJavaException("java/io/IOException", ex.what()); } catch(const std::bad_alloc& ex) { - current = JOutOfMemoryError::create(ex.what()); + setNewJavaException("java/lang/OutOfMemoryError", ex.what()); } catch(const std::out_of_range& ex) { - current = JArrayIndexOutOfBoundsException::create(ex.what()); + setNewJavaException("java/lang/ArrayIndexOutOfBoundsException", ex.what()); } catch(const std::system_error& ex) { - current = JCppSystemErrorException::create(ex); + setCppSystemErrorExceptionInJava(ex); } catch(const std::runtime_error& ex) { - current = JRuntimeException::create(ex.what()); + setNewJavaException("java/lang/RuntimeException", ex.what()); } catch(const std::exception& ex) { - current = JCppException::create(ex.what()); + setNewJavaException("com/facebook/jni/CppException", ex.what()); } catch(const char* msg) { - current = JUnknownCppException::create(msg); + setNewJavaException(CommonJniExceptions::getUnknownCppExceptionClass(), msg); } catch(...) { - current = JUnknownCppException::create(); - } - if (previous) { - current->initCause(previous); + setJavaExceptionAndAbortOnFailure(CommonJniExceptions::getUnknownCppExceptionObject()); } - previous = current; - }; - - try { - denest(func); - setJavaExceptionAndAbortOnFailure(previous); - } catch (std::exception& e) { - FBLOGE("unexpected exception in translatePendingCppExceptionToJavaException: %s", e.what()); - // rethrow the exception and let the noexcept handling abort. - throw; - } catch (...) { - FBLOGE("unexpected exception in translatePendingCppExceptionToJavaException"); - throw; + } catch(...) { + // This block aborts the program, if something bad happens when handling exceptions, thus + // keeping this function noexcept. + std::abort(); } } @@ -234,41 +310,79 @@ void translatePendingCppExceptionToJavaException() noexcept { const std::string JniException::kExceptionMessageFailure_ = "Unable to get exception message."; -JniException::JniException() : JniException(JRuntimeException::create()) { } +JniException::JniException() : JniException(CommonJniExceptions::getRuntimeExceptionObject()) { } -JniException::JniException(alias_ref throwable) : isMessageExtracted_(false) { - throwable_ = make_global(throwable); +JniException::JniException(jthrowable throwable) : isMessageExtracted_(false) { + assertIfExceptionsNotInitialized(); + throwableGlobalRef_ = static_cast(internal::getEnv()->NewGlobalRef(throwable)); + if (!throwableGlobalRef_) { + throw std::bad_alloc(); + } } JniException::JniException(JniException &&rhs) - : throwable_(std::move(rhs.throwable_)), + : throwableGlobalRef_(std::move(rhs.throwableGlobalRef_)), what_(std::move(rhs.what_)), isMessageExtracted_(rhs.isMessageExtracted_) { + rhs.throwableGlobalRef_ = nullptr; } JniException::JniException(const JniException &rhs) : what_(rhs.what_), isMessageExtracted_(rhs.isMessageExtracted_) { - throwable_ = make_global(rhs.throwable_); + JNIEnv* env = internal::getEnv(); + if (rhs.getThrowable()) { + throwableGlobalRef_ = static_cast(env->NewGlobalRef(rhs.getThrowable())); + if (!throwableGlobalRef_) { + throw std::bad_alloc(); + } + } else { + throwableGlobalRef_ = nullptr; + } } -JniException::~JniException() { - ThreadScope ts; - throwable_.reset(); +JniException::~JniException() noexcept { + if (throwableGlobalRef_) { + internal::getEnv()->DeleteGlobalRef(throwableGlobalRef_); + } } -local_ref JniException::getThrowable() const noexcept { - return make_local(throwable_); +jthrowable JniException::getThrowable() const noexcept { + return throwableGlobalRef_; } // TODO 6900503: consider making this thread-safe. void JniException::populateWhat() const noexcept { - ThreadScope ts; + JNIEnv* env = internal::getEnv(); + + jmethodID toStringMID = env->GetMethodID( + CommonJniExceptions::getThrowableClass(), + "toString", + "()Ljava/lang/String;"); + jstring messageJString = (jstring) env->CallObjectMethod( + throwableGlobalRef_, + toStringMID); + + isMessageExtracted_ = true; + + if (env->ExceptionCheck()) { + env->ExceptionClear(); + what_ = kExceptionMessageFailure_; + return; + } + + const char* chars = env->GetStringUTFChars(messageJString, nullptr); + if (!chars) { + what_ = kExceptionMessageFailure_; + return; + } + try { - what_ = throwable_->toString(); - isMessageExtracted_ = true; + what_ = std::string(chars); } catch(...) { what_ = kExceptionMessageFailure_; } + + env->ReleaseStringUTFChars(messageJString, chars); } const char* JniException::what() const noexcept { @@ -279,7 +393,7 @@ const char* JniException::what() const noexcept { } void JniException::setJavaException() const noexcept { - setJavaExceptionAndAbortOnFailure(throwable_); + setJavaExceptionAndAbortOnFailure(throwableGlobalRef_); } }} diff --git a/ReactAndroid/src/main/jni/first-party/fb/jni/LocalString.cpp b/ReactAndroid/src/main/jni/first-party/fb/jni/LocalString.cpp index 2e197f075983..1a0e8d703a12 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/jni/LocalString.cpp +++ b/ReactAndroid/src/main/jni/first-party/fb/jni/LocalString.cpp @@ -156,7 +156,7 @@ void utf8ToModifiedUTF8(const uint8_t* utf8, size_t len, uint8_t* modified, size modified[j++] = '\0'; } -std::string modifiedUTF8ToUTF8(const uint8_t* modified, size_t len) noexcept { +std::string modifiedUTF8ToUTF8(const uint8_t* modified, size_t len) { // Converting from modified utf8 to utf8 will always shrink, so this will always be sufficient std::string utf8(len, 0); size_t j = 0; @@ -230,7 +230,7 @@ size_t utf16toUTF8Length(const uint16_t* utf16String, size_t utf16StringLen) { return utf8StringLen; } -std::string utf16toUTF8(const uint16_t* utf16String, size_t utf16StringLen) noexcept { +std::string utf16toUTF8(const uint16_t* utf16String, size_t utf16StringLen) { if (!utf16String || utf16StringLen <= 0) { return ""; } diff --git a/ReactAndroid/src/main/jni/first-party/fb/jni/OnLoad.cpp b/ReactAndroid/src/main/jni/first-party/fb/jni/OnLoad.cpp index 0334729c8e20..d50416133788 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/jni/OnLoad.cpp +++ b/ReactAndroid/src/main/jni/first-party/fb/jni/OnLoad.cpp @@ -10,13 +10,10 @@ #include #include #include -#include using namespace facebook::jni; void initialize_fbjni() { CountableOnLoad(Environment::current()); HybridDataOnLoad(); - JNativeRunnable::OnLoad(); - ThreadScope::OnLoad(); } diff --git a/ReactAndroid/src/main/jni/first-party/fb/jni/fbjni.cpp b/ReactAndroid/src/main/jni/first-party/fb/jni/fbjni.cpp index de9423c20192..e2bd795fab64 100644 --- a/ReactAndroid/src/main/jni/first-party/fb/jni/fbjni.cpp +++ b/ReactAndroid/src/main/jni/first-party/fb/jni/fbjni.cpp @@ -26,6 +26,7 @@ jint initialize(JavaVM* vm, std::function&& init_fn) noexcept { std::call_once(flag, [vm] { try { Environment::initialize(vm); + internal::initExceptionHelpers(); } catch (std::exception& ex) { error_occured = true; try { @@ -57,9 +58,6 @@ jint initialize(JavaVM* vm, std::function&& init_fn) noexcept { alias_ref findClassStatic(const char* name) { const auto env = internal::getEnv(); - if (!env) { - throw std::runtime_error("Unable to retrieve JNIEnv*."); - } auto cls = env->FindClass(name); FACEBOOK_JNI_THROW_EXCEPTION_IF(!cls); auto leaking_ref = (jclass)env->NewGlobalRef(cls); @@ -69,9 +67,6 @@ alias_ref findClassStatic(const char* name) { local_ref findClassLocal(const char* name) { const auto env = internal::getEnv(); - if (!env) { - throw std::runtime_error("Unable to retrieve JNIEnv*."); - } auto cls = env->FindClass(name); FACEBOOK_JNI_THROW_EXCEPTION_IF(!cls); return adopt_local(cls);