diff --git a/libs/fbjni/cxx/fbjni/ByteBuffer.cpp b/libs/fbjni/cxx/fbjni/ByteBuffer.cpp index bf8f6c55e..c0cd93eb0 100644 --- a/libs/fbjni/cxx/fbjni/ByteBuffer.cpp +++ b/libs/fbjni/cxx/fbjni/ByteBuffer.cpp @@ -12,25 +12,52 @@ namespace facebook { namespace jni { -namespace { -local_ref createEmpty() { - static auto cls = JByteBuffer::javaClassStatic(); - static auto meth = cls->getStaticMethod("allocateDirect"); - return meth(cls, 0); -} -} - void JBuffer::rewind() const { static auto meth = javaClassStatic()->getMethod()>("rewind"); meth(self()); } +void* JBuffer::getDirectAddress() const { + if (!self()) { + throwNewJavaException("java/lang/NullPointerException", "java.lang.NullPointerException"); + } + void* addr = Environment::current()->GetDirectBufferAddress(self()); + FACEBOOK_JNI_THROW_PENDING_EXCEPTION(); + if (!addr) { + throw std::runtime_error( + isDirect() ? + "Attempt to get direct bytes of non-direct buffer." : + "Error getting direct bytes of buffer."); + } + return addr; +} + +size_t JBuffer::getDirectCapacity() const { + if (!self()) { + throwNewJavaException("java/lang/NullPointerException", "java.lang.NullPointerException"); + } + int size = Environment::current()->GetDirectBufferCapacity(self()); + FACEBOOK_JNI_THROW_PENDING_EXCEPTION(); + if (size < 0) { + throw std::runtime_error( + isDirect() ? + "Attempt to get direct size of non-direct buffer." : + "Error getting direct size of buffer."); + } + return static_cast(size); +} + +bool JBuffer::isDirect() const { + static auto meth = javaClassStatic()->getMethod("isDirect"); + return meth(self()); +} + local_ref JByteBuffer::wrapBytes(uint8_t* data, size_t size) { // env->NewDirectByteBuffer requires that size is positive. Android's // dalvik returns an invalid result and Android's art aborts if size == 0. // Workaround this by using a slow path through Java in that case. if (!size) { - return createEmpty(); + return allocateDirect(0); } auto res = adopt_local(static_cast(Environment::current()->NewDirectByteBuffer(data, size))); FACEBOOK_JNI_THROW_PENDING_EXCEPTION(); @@ -40,39 +67,10 @@ local_ref JByteBuffer::wrapBytes(uint8_t* data, size_t size) { return res; } -uint8_t* JByteBuffer::getDirectBytes() const { - if (!self()) { - throwNewJavaException("java/lang/NullPointerException", "java.lang.NullPointerException"); - } - void* bytes = Environment::current()->GetDirectBufferAddress(self()); - FACEBOOK_JNI_THROW_PENDING_EXCEPTION(); - if (!bytes) { - throw std::runtime_error( - isDirect() ? - "Attempt to get direct bytes of non-direct byte buffer." : - "Error getting direct bytes of byte buffer."); - } - return static_cast(bytes); -} - -size_t JByteBuffer::getDirectSize() const { - if (!self()) { - throwNewJavaException("java/lang/NullPointerException", "java.lang.NullPointerException"); - } - int size = Environment::current()->GetDirectBufferCapacity(self()); - FACEBOOK_JNI_THROW_PENDING_EXCEPTION(); - if (size < 0) { - throw std::runtime_error( - isDirect() ? - "Attempt to get direct size of non-direct byte buffer." : - "Error getting direct size of byte buffer."); - } - return static_cast(size); -} - -bool JByteBuffer::isDirect() const { - static auto meth = javaClassStatic()->getMethod("isDirect"); - return meth(self()); +local_ref JByteBuffer::allocateDirect(jint size) { + static auto cls = JByteBuffer::javaClassStatic(); + static auto meth = cls->getStaticMethod("allocateDirect"); + return meth(cls, size); } }} diff --git a/libs/fbjni/cxx/fbjni/ByteBuffer.h b/libs/fbjni/cxx/fbjni/ByteBuffer.h index d4d4fa0a8..85c6177db 100644 --- a/libs/fbjni/cxx/fbjni/ByteBuffer.h +++ b/libs/fbjni/cxx/fbjni/ByteBuffer.h @@ -17,6 +17,9 @@ public: static constexpr const char* kJavaDescriptor = "Ljava/nio/Buffer;"; void rewind() const; + bool isDirect() const; + void* getDirectAddress() const; + size_t getDirectCapacity() const; }; // JNI's NIO support has some awkward preconditions and error reporting. This @@ -26,11 +29,15 @@ class JByteBuffer : public JavaClass { static constexpr const char* kJavaDescriptor = "Ljava/nio/ByteBuffer;"; static local_ref wrapBytes(uint8_t* data, size_t size); + static local_ref allocateDirect(jint size); - bool isDirect() const; + uint8_t* getDirectBytes() const { + return static_cast(getDirectAddress()); + } - uint8_t* getDirectBytes() const; - size_t getDirectSize() const; + size_t getDirectSize() const { + return getDirectCapacity(); + } }; }} diff --git a/libs/fbjni/cxx/fbjni/detail/Boxed.h b/libs/fbjni/cxx/fbjni/detail/Boxed.h index b2621dd3f..87b79e51e 100644 --- a/libs/fbjni/cxx/fbjni/detail/Boxed.h +++ b/libs/fbjni/cxx/fbjni/detail/Boxed.h @@ -56,6 +56,14 @@ DEFINE_BOXED_PRIMITIVE(double, Double) #undef DEFINE_BOXED_PRIMITIVE +template +inline typename std::enable_if< + (std::is_same::value || std::is_same::value) && !std::is_same::value, + local_ref +>::type autobox(T val) { + return JLong::valueOf(val); +} + struct JVoid : public jni::JavaClass { static auto constexpr kJavaDescriptor = "Ljava/lang/Void;"; }; diff --git a/libs/fbjni/cxx/fbjni/detail/CoreClasses-inl.h b/libs/fbjni/cxx/fbjni/detail/CoreClasses-inl.h index 87fa7b6e3..d4860aeee 100644 --- a/libs/fbjni/cxx/fbjni/detail/CoreClasses-inl.h +++ b/libs/fbjni/cxx/fbjni/detail/CoreClasses-inl.h @@ -48,6 +48,11 @@ inline void JObject::setFieldValue(JField field, T value) noexcept { field.set(self(), value); } +template +inline void JObject::setFieldValue(JField field, alias_ref value) noexcept { + setFieldValue(field, value.get()); +} + inline std::string JObject::toString() const { static const auto method = findClassLocal("java/lang/Object")->getMethod("toString"); @@ -288,6 +293,11 @@ inline void JClass::setStaticFieldValue(JStaticField field, T value) noexcept field.set(self(), value); } +template +inline void JClass::setStaticFieldValue(JStaticField field, alias_ref value) noexcept { + setStaticFieldValue(field, value.get()); +} + template inline local_ref JClass::newObject( JConstructor constructor, @@ -444,7 +454,6 @@ local_ref::javaobject> adopt_local_array(jobjectArray re template auto JPrimitiveArray::getRegion(jsize start, jsize length) -> std::unique_ptr { - using T = typename jtype_traits::entry_type; auto buf = std::unique_ptr{new T[length]}; getRegion(start, length, buf.get()); return buf; @@ -513,6 +522,8 @@ class PinnedCriticalAlloc { T** elements, size_t* size, jboolean* isCopy) { + (void)start; + (void)length; const auto env = Environment::current(); *elements = static_cast(env->GetPrimitiveArrayCritical(array.get(), isCopy)); FACEBOOK_JNI_THROW_EXCEPTION_IF(!elements); @@ -524,6 +535,8 @@ class PinnedCriticalAlloc { jint start, jint size, jint mode) { + (void)start; + (void)size; const auto env = Environment::current(); env->ReleasePrimitiveArrayCritical(array.get(), elements, mode); } diff --git a/libs/fbjni/cxx/fbjni/detail/CoreClasses.h b/libs/fbjni/cxx/fbjni/detail/CoreClasses.h index 18f08cee9..bb5a21c1e 100644 --- a/libs/fbjni/cxx/fbjni/detail/CoreClasses.h +++ b/libs/fbjni/cxx/fbjni/detail/CoreClasses.h @@ -27,6 +27,13 @@ namespace jni { class JClass; class JObject; +namespace detail { + +/// Lookup a class by name. This should only be used internally. +jclass findClass(JNIEnv* env, const char* name); + +} + /// Lookup a class by name. Note this functions returns an alias_ref that /// points to a leaked global reference. This is appropriate for classes /// that are never unloaded (which is any class in an Android app and most @@ -83,7 +90,7 @@ bool isSameObject(alias_ref lhs, alias_ref rhs) noexcept; // // While users of a JavaClass-type can lookup methods and fields through the // underlying JClass, those calls can only be checked at runtime. It is recommended -// that the JavaClass-type instead explicitly expose its methods as in the example +// that the JavaClass-type instead explicitly expose it's methods as in the example // above. namespace detail { @@ -114,10 +121,12 @@ public: template local_ref getFieldValue(JField field) const noexcept; - /// Set the value of field. Any Java type is accepted, including the primitive types - /// and raw reference types. + /// Set the value of field. Any Java type is accepted. template void setFieldValue(JField field, T value) noexcept; + template(), T>::type> + void setFieldValue(JField field, alias_ref value) noexcept; /// Convenience method to create a std::string representing the object std::string toString() const; @@ -233,11 +242,18 @@ class JClass : public JavaClass { /// makeNativeMethod("nativeMethodWithExplicitDescriptor", /// "(Lcom/facebook/example/MyClass;)V", /// methodWithExplicitDescriptor), + /// makeCriticalNativeMethod_DO_NOT_USE_OR_YOU_WILL_BE_FIRED("criticalNativeMethodWithAutomaticDescriptor", + /// criticalNativeMethodWithAutomaticDescriptor), + /// makeCriticalNativeMethod_DO_NOT_USE_OR_YOU_WILL_BE_FIRED("criticalNativeMethodWithExplicitDescriptor", + /// "(IIF)Z", + /// criticalNativeMethodWithExplicitDescriptor), /// }); /// /// By default, C++ exceptions raised will be converted to Java exceptions. /// To avoid this and get the "standard" JNI behavior of a crash when a C++ /// exception is crashing out of the JNI method, declare the method noexcept. + /// This does NOT apply to critical native methods, where exceptions causes + /// a crash. void registerNatives(std::initializer_list methods); /// Check to see if the class is assignable from another class @@ -287,10 +303,12 @@ class JClass : public JavaClass { template local_ref getStaticFieldValue(JStaticField field) noexcept; - /// Set the value of field. Any Java type is accepted, including the primitive types - /// and raw reference types. + /// Set the value of field. Any Java type is accepted. template void setStaticFieldValue(JStaticField field, T value) noexcept; + template(), T>::type> + void setStaticFieldValue(JStaticField field, alias_ref value) noexcept; /// Allocates a new object and invokes the specified constructor template diff --git a/libs/fbjni/cxx/fbjni/detail/Exceptions.h b/libs/fbjni/cxx/fbjni/detail/Exceptions.h index 7eb01256d..64c2489e3 100644 --- a/libs/fbjni/cxx/fbjni/detail/Exceptions.h +++ b/libs/fbjni/cxx/fbjni/detail/Exceptions.h @@ -64,7 +64,7 @@ class JCppException : public JavaClass { class JniException : public std::exception { public: JniException(); - ~JniException(); + ~JniException() override; explicit JniException(alias_ref throwable); @@ -74,7 +74,7 @@ class JniException : public std::exception { local_ref getThrowable() const noexcept; - virtual const char* what() const noexcept; + const char* what() const noexcept override; void setJavaException() const noexcept; diff --git a/libs/fbjni/cxx/fbjni/detail/Hybrid.h b/libs/fbjni/cxx/fbjni/detail/Hybrid.h index 86354d202..75b14884d 100644 --- a/libs/fbjni/cxx/fbjni/detail/Hybrid.h +++ b/libs/fbjni/cxx/fbjni/detail/Hybrid.h @@ -31,7 +31,7 @@ class HybridDestructor : public JavaClass { public: static auto constexpr kJavaDescriptor = "Lcom/facebook/jni/HybridData$Destructor;"; - detail::BaseHybridClass* getNativePointer(); + detail::BaseHybridClass* getNativePointer() const; void setNativePointer(std::unique_ptr new_value); }; @@ -134,7 +134,7 @@ public: // This will reach into the java object and extract the C++ instance from // the mHybridData and return it. - T* cthis(); + T* cthis() const; friend class HybridClass; friend T; @@ -162,7 +162,7 @@ protected: using detail::HybridTraits::CxxBase::CxxBase; static void registerHybrid(std::initializer_list methods) { - javaClassStatic()->registerNatives(methods); + javaClassLocal()->registerNatives(methods); } static local_ref makeHybridData(std::unique_ptr cxxPart) { @@ -235,11 +235,13 @@ public: // particular java class) to be hoisted to a common function. If // mapException returns, then the std::exception will be translated // to Java. - static void mapException(const std::exception& ex) {} + static void mapException(const std::exception& ex) { + (void)ex; + } }; template -inline T* HybridClass::JavaPart::cthis() { +inline T* HybridClass::JavaPart::cthis() const { detail::BaseHybridClass* result = 0; static bool isHybrid = detail::HybridClassBase::isHybridClassBase(this->getClass()); if (isHybrid) { diff --git a/libs/fbjni/cxx/fbjni/detail/Meta.h b/libs/fbjni/cxx/fbjni/detail/Meta.h index 28228fce4..e03ba7d78 100644 --- a/libs/fbjni/cxx/fbjni/detail/Meta.h +++ b/libs/fbjni/cxx/fbjni/detail/Meta.h @@ -33,7 +33,7 @@ namespace facebook { namespace jni { -// This will get the reflected Java Method from the method_id, get its invoke +// This will get the reflected Java Method from the method_id, get it's invoke // method, and call the method via that. This shouldn't ever be needed, but // Android 6.0 crashes when calling a method on a java.lang.Proxy via jni. template diff --git a/libs/fbjni/cxx/fbjni/detail/MetaConvert.h b/libs/fbjni/cxx/fbjni/detail/MetaConvert.h index 9eb1cfb94..af8075a0f 100644 --- a/libs/fbjni/cxx/fbjni/detail/MetaConvert.h +++ b/libs/fbjni/cxx/fbjni/detail/MetaConvert.h @@ -67,6 +67,25 @@ struct Convert { } }; +// Sometimes (64-bit Android) jlong is "long long", but int64_t is "long". +// Allow int64_t to work as jlong. +template +struct Convert::value || std::is_same::value) && !std::is_same::value + >::type> { + typedef jlong jniType; + static T fromJni(jniType t) { + return t; + } + static jniType toJniRet(T t) { + return t; + } + static jniType toCall(T t) { + return t; + } +}; + // convert to alias_ref from T template struct Convert> { diff --git a/libs/fbjni/cxx/fbjni/detail/References-inl.h b/libs/fbjni/cxx/fbjni/detail/References-inl.h index fc38ad208..785846456 100644 --- a/libs/fbjni/cxx/fbjni/detail/References-inl.h +++ b/libs/fbjni/cxx/fbjni/detail/References-inl.h @@ -174,6 +174,29 @@ operator!=(const T1& a, const T2& b) { return !(a == b); } +template +inline enable_if_t(), bool> +operator==(const T1& a, std::nullptr_t) { + return getPlainJniReference(a) == nullptr; +} + +template +inline enable_if_t(), bool> +operator==(std::nullptr_t, const T1& a) { + return nullptr == getPlainJniReference(a); +} + +template +inline enable_if_t(), bool> +operator!=(const T1& a, std::nullptr_t) { + return !(a == nullptr); +} + +template +inline enable_if_t(), bool> +operator!=(std::nullptr_t, const T1& a) { + return !(nullptr == getPlainJniReference(a)); +} // base_owned_ref /////////////////////////////////////////////////////////////////////// @@ -185,7 +208,9 @@ inline base_owned_ref::base_owned_ref() noexcept template inline base_owned_ref::base_owned_ref(std::nullptr_t t) noexcept : base_owned_ref(static_cast(nullptr)) -{} +{ + (void)t; +} template inline base_owned_ref::base_owned_ref(const base_owned_ref& other) diff --git a/libs/fbjni/cxx/fbjni/detail/References.cpp b/libs/fbjni/cxx/fbjni/detail/References.cpp index 11d7561c6..f1abe939d 100644 --- a/libs/fbjni/cxx/fbjni/detail/References.cpp +++ b/libs/fbjni/cxx/fbjni/detail/References.cpp @@ -29,12 +29,23 @@ namespace { #ifdef __ANDROID__ int32_t getAndroidApiLevel() { - auto cls = findClassLocal("android/os/Build$VERSION"); - auto fld = cls->getStaticField("SDK_INT"); - if (fld) { - return cls->getStaticFieldValue(fld); + // This is called from the static local initializer in + // isObjectRefType(), and creating fbjni references can call + // isObjectRefType(). So, to avoid recursively entering the block + // where the static is initialized (which is undefined behavior), we + // avoid using standard fbjni references here. + + JNIEnv* env = Environment::current(); + jclass cls = detail::findClass(env, "android/os/Build$VERSION"); + jfieldID field = env->GetStaticFieldID(cls, "SDK_INT", + jtype_traits::descriptor().c_str()); + if (!field) { + env->DeleteLocalRef(cls); } - return 0; + FACEBOOK_JNI_THROW_EXCEPTION_IF(!field); + int32_t ret = env->GetStaticIntField(cls, field); + env->DeleteLocalRef(cls); + return ret; } bool doesGetObjectRefTypeWork() { @@ -54,13 +65,15 @@ bool doesGetObjectRefTypeWork() { } bool isObjectRefType(jobject reference, jobjectRefType refType) { + // null-check first so that we short-circuit during (safe) global + // constructors, where we won't have an Environment::current() yet + if (!reference) { + return true; + } -// TODO Rather than setting it true, use doesGetObjectRefTypeWork(). But it -// causes sample app to freeze - static bool getObjectRefTypeWorks = true; + static bool getObjectRefTypeWorks = doesGetObjectRefTypeWork(); return - !reference || !getObjectRefTypeWorks || Environment::current()->GetObjectRefType(reference) == refType; } diff --git a/libs/fbjni/cxx/fbjni/detail/References.h b/libs/fbjni/cxx/fbjni/detail/References.h index ca226eaee..f61020a0c 100644 --- a/libs/fbjni/cxx/fbjni/detail/References.h +++ b/libs/fbjni/cxx/fbjni/detail/References.h @@ -249,6 +249,26 @@ template enable_if_t() && IsNonWeakReference(), bool> operator!=(const T1& a, const T2& b); +/** + * Compare references against nullptr + */ +template +enable_if_t(), bool> +operator==(const T1& a, std::nullptr_t); + +template +enable_if_t(), bool> +operator==(std::nullptr_t, const T1& a); + +template +enable_if_t(), bool> +operator!=(const T1& a, std::nullptr_t); + +template +enable_if_t(), bool> +operator!=(std::nullptr_t, const T1& a); + + template class base_owned_ref { public: diff --git a/libs/fbjni/cxx/fbjni/detail/Registration-inl.h b/libs/fbjni/cxx/fbjni/detail/Registration-inl.h index df1c83409..8f74ad4eb 100644 --- a/libs/fbjni/cxx/fbjni/detail/Registration-inl.h +++ b/libs/fbjni/cxx/fbjni/detail/Registration-inl.h @@ -143,6 +143,25 @@ inline std::string makeDescriptor(R (C::*)(Args... args)) { return jmethod_traits_from_cxx::descriptor(); } +template +template +JNI_ENTRY_POINT R CriticalMethod::call(alias_ref, Args... args) noexcept { + static_assert( + IsJniPrimitive() || std::is_void(), + "Critical Native Methods may only return primitive JNI types, or void."); + static_assert( + AreJniPrimitives(), + "Critical Native Methods may only use primitive JNI types as parameters"); + + return func(std::forward(args)...); +} + +template +template +inline std::string CriticalMethod::desc() { + return makeDescriptor(call); +} + } }} diff --git a/libs/fbjni/cxx/fbjni/detail/Registration.h b/libs/fbjni/cxx/fbjni/detail/Registration.h index 9f1223962..229a2d4a8 100644 --- a/libs/fbjni/cxx/fbjni/detail/Registration.h +++ b/libs/fbjni/cxx/fbjni/detail/Registration.h @@ -44,6 +44,18 @@ std::string makeDescriptor(R (*func)(alias_ref, Args... args)); template std::string makeDescriptor(R (C::*method0)(Args... args)); +template +struct CriticalMethod; + +template +struct CriticalMethod { + template + static R call(alias_ref, Args... args) noexcept; + + template + inline static std::string desc(); +}; + } // We have to use macros here, because the func needs to be used @@ -66,6 +78,91 @@ std::string makeDescriptor(R (C::*method0)(Args... args)); #define makeNativeMethodN(a, b, c, count, ...) makeNativeMethod ## count #define makeNativeMethod(...) makeNativeMethodN(__VA_ARGS__, 3, 2)(__VA_ARGS__) + +// FAST CALLS / CRITICAL CALLS +// Android up to and including v7 supports "fast calls" by prefixing the method +// signature with an exclamation mark. +// Android v8+ supports fast calls by annotating methods: +// https://source.android.com/devices/tech/dalvik/improvements#faster-native-methods +// +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// +// "Fast" calls are only on the order of a few dozen NANO-seconds faster than +// regular JNI calls. If your method does almost aaanything of consequence - if +// you loop, if you write to a log, if you call another method, if you even +// simply allocate or deallocate - then the method body will significantly +// outweigh the method overhead. +// +// The difference between a regular JNI method and a "FastJNI" method (as +// they're called inside the runtime) is that a FastJNI method doesn't mark the +// thread as executing native code, and by skipping that avoids the locking and +// thread state check overhead of interacting with the Garbage Collector. +// +// To understand why this is dangerous, you need to understand a bit about the +// GC. In order to perform its work the GC needs to have at least one (usually +// two in modern implementations) "stop the world" checkpoints where it can +// guarantee that all managed-code execution is paused. The VM performs these +// checkpoints at allocations, method boundaries, and each backward branch (ie +// anytime you loop). When the GC wants to run, it will signal to all managed +// threads that they should pause at the next checkpoint, and then it will wait +// for every thread in the system to transition from the "runnable" state into a +// "waiting" state. Once every thread has stopped, the GC thread can perform the +// work it needs to and then it will trigger the execution threads to resume. +// +// JNI methods fit neatly into the above paradigm: They're still methods, so +// they perform GC checkpoints at method entry and method exit. JNI methods also +// perform checkpoints at any JNI boundary crossing - ie, any time you call +// GetObjectField etc. Because access to managed objects from native code is +// tightly controlled, the VM is able to mark threads executing native methods +// into a special "native" state which the GC is able to ignore: It knows they +// can't touch managed objects (without hitting a checkpoint) so it doesn't care +// about them. +// +// JNI critical methods don't perform that "runnable" -> "native" thread state +// transition. Skipping that transition allows them to shave about 20ns off +// their total execution time, but it means that the GC has to wait for them to +// complete before it can move forward. If a critical method begins blocking, +// say on a long loop, or an I/O operation, or on perhaps a mutex, then the GC +// will also block, and because the GC is blocking the entire rest of the VM +// (which is waiting on the GC) will block. If the critical method is blocking +// on a mutex that's already held by the GC - for example, the VM's internal +// weak_globals_lock_ which guards modifications to the weak global reference +// table (and is required in order to create or free a weak_ref<>) - then you +// have a system-wide deadlock. + +// prefixes a JNI method signature as android "fast call". +#if defined(__ANDROID__) && defined(FBJNI_WITH_FAST_CALLS) +#define FBJNI_PREFIX_FAST_CALL(desc) (std::string{"!"} + desc) +#else +#define FBJNI_PREFIX_FAST_CALL(desc) (desc) +#endif + +#define makeCriticalNativeMethod3(name, desc, func) \ + makeNativeMethod3( \ + name, \ + FBJNI_PREFIX_FAST_CALL(desc), \ + ::facebook::jni::detail::CriticalMethod::call<&func>) + +#define makeCriticalNativeMethod2(name, func) \ + makeCriticalNativeMethod3( \ + name, \ + ::facebook::jni::detail::CriticalMethod::desc<&func>(), \ + func) + +#define makeCriticalNativeMethodN(a, b, c, count, ...) makeCriticalNativeMethod ## count + +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// YOU ALMOST CERTAINLY DO NOT NEED THIS AND IT IS DANGEROUS. +// See above for an explanation. +#define makeCriticalNativeMethod_DO_NOT_USE_OR_YOU_WILL_BE_FIRED(...) makeCriticalNativeMethodN(__VA_ARGS__, 3, 2)(__VA_ARGS__) + }} #include "Registration-inl.h" diff --git a/libs/fbjni/cxx/fbjni/detail/TypeTraits.h b/libs/fbjni/cxx/fbjni/detail/TypeTraits.h index 229ed4aaf..941e98e78 100644 --- a/libs/fbjni/cxx/fbjni/detail/TypeTraits.h +++ b/libs/fbjni/cxx/fbjni/detail/TypeTraits.h @@ -69,6 +69,25 @@ constexpr bool IsJniPrimitive() { return is_jni_primitive::value; } +/// Metafunction to determine whether a series of types are all primitive JNI types. +template +struct are_jni_primitives; + +template +struct are_jni_primitives : + std::integral_constant::value && are_jni_primitives::value> {}; + +template<> +struct are_jni_primitives<> : std::integral_constant {}; + +/// Helper to simplify use of are_jni_primitives +template +constexpr bool AreJniPrimitives() { + return are_jni_primitives::value; +} + + /// Metafunction to determine whether a type is a JNI array of primitives or not template struct is_jni_primitive_array : diff --git a/libs/fbjni/cxx/fbjni/fbjni.cpp b/libs/fbjni/cxx/fbjni/fbjni.cpp index 879b80290..7cee456f1 100644 --- a/libs/fbjni/cxx/fbjni/fbjni.cpp +++ b/libs/fbjni/cxx/fbjni/fbjni.cpp @@ -52,26 +52,29 @@ jint initialize(JavaVM* vm, std::function&& init_fn) noexcept { return JNI_VERSION_1_6; } -alias_ref findClassStatic(const char* name) { - const auto env = detail::currentOrNull(); +namespace detail { + +jclass findClass(JNIEnv* env, const char* name) { if (!env) { throw std::runtime_error("Unable to retrieve JNIEnv*."); } - local_ref cls = adopt_local(env->FindClass(name)); + jclass cls = env->FindClass(name); FACEBOOK_JNI_THROW_EXCEPTION_IF(!cls); - auto leaking_ref = (jclass)env->NewGlobalRef(cls.get()); - FACEBOOK_JNI_THROW_EXCEPTION_IF(!leaking_ref); - return wrap_alias(leaking_ref); + return cls; +} + } local_ref findClassLocal(const char* name) { - const auto env = detail::currentOrNull(); - 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); + return adopt_local(detail::findClass(detail::currentOrNull(), name)); +} + +alias_ref findClassStatic(const char* name) { + JNIEnv* env = detail::currentOrNull(); + auto cls = adopt_local(detail::findClass(env, name)); + auto leaking_ref = (jclass)env->NewGlobalRef(cls.get()); + FACEBOOK_JNI_THROW_EXCEPTION_IF(!leaking_ref); + return wrap_alias(leaking_ref); } @@ -104,7 +107,7 @@ local_ref make_jstring(const char* utf8) { // The only difference between utf8 and modifiedUTF8 is in encoding 4-byte UTF8 chars // and '\0' that is encoded on 2 bytes. // - // Since modifiedUTF8-encoded string can be no shorter than its UTF8 conterpart we + // Since modifiedUTF8-encoded string can be no shorter than it's UTF8 conterpart we // know that if those two strings are of the same length we don't need to do any // conversion -> no 4-byte chars nor '\0'. result = env->NewStringUTF(utf8); @@ -194,7 +197,7 @@ DEFINE_PRIMITIVE_METHODS(jdouble, Double, double) namespace detail { -detail::BaseHybridClass* HybridDestructor::getNativePointer() { +detail::BaseHybridClass* HybridDestructor::getNativePointer() const { static auto pointerField = javaClassStatic()->getField("mNativePointer"); auto* value = reinterpret_cast(getFieldValue(pointerField)); if (!value) { diff --git a/libs/fbjni/java/com/facebook/jni/DestructorThread.java b/libs/fbjni/java/com/facebook/jni/DestructorThread.java index 23ea64413..45dc805f0 100644 --- a/libs/fbjni/java/com/facebook/jni/DestructorThread.java +++ b/libs/fbjni/java/com/facebook/jni/DestructorThread.java @@ -33,7 +33,7 @@ public class DestructorThread { private Destructor next; private Destructor previous; - Destructor(Object referent) { + public Destructor(Object referent) { super(referent, sReferenceQueue); sDestructorStack.push(this); } @@ -43,16 +43,16 @@ public class DestructorThread { } /** Callback which is invoked when the original object has been garbage collected. */ - abstract void destruct(); + protected abstract void destruct(); } /** A list to keep all active Destructors in memory confined to the Destructor thread. */ - private static DestructorList sDestructorList; + private static final DestructorList sDestructorList; /** A thread safe stack where new Destructors are placed before being add to sDestructorList. */ - private static DestructorStack sDestructorStack; + private static final DestructorStack sDestructorStack; - private static ReferenceQueue sReferenceQueue; - private static Thread sThread; + private static final ReferenceQueue sReferenceQueue; + private static final Thread sThread; static { sDestructorStack = new DestructorStack(); @@ -86,14 +86,14 @@ public class DestructorThread { private static class Terminus extends Destructor { @Override - void destruct() { + protected void destruct() { throw new IllegalStateException("Cannot destroy Terminus Destructor."); } } /** This is a thread safe, lock-free Treiber-like Stack of Destructors. */ private static class DestructorStack { - private AtomicReference mHead = new AtomicReference<>(); + private final AtomicReference mHead = new AtomicReference<>(); public void push(Destructor newHead) { Destructor oldHead; @@ -115,7 +115,7 @@ public class DestructorThread { /** A doubly-linked list of Destructors. */ private static class DestructorList { - private Destructor mHead; + private final Destructor mHead; public DestructorList() { mHead = new Terminus(); diff --git a/libs/fbjni/java/com/facebook/jni/HybridData.java b/libs/fbjni/java/com/facebook/jni/HybridData.java index 9a36b9782..d56c788ad 100644 --- a/libs/fbjni/java/com/facebook/jni/HybridData.java +++ b/libs/fbjni/java/com/facebook/jni/HybridData.java @@ -65,7 +65,7 @@ public class HybridData { } @Override - void destruct() { + protected final void destruct() { // When invoked from the DestructorThread instead of resetNative, // the DestructorThread has exclusive ownership of the HybridData // so synchronization is not necessary.