Sync fbjni copy with upstream

Summary: Merge upstream fbjni into our copy.

Reviewed By: priteshrnandgaonkar

Differential Revision: D16828183

fbshipit-source-id: 720f7411d4327870c01d3f70e9d2b03909362795

libs/fbjni/cxx/fbjni/ByteBuffer.cpp

@@ -12,25 +12,52 @@
 namespace facebook {
 namespace jni {
 
-namespace {
-local_ref<JByteBuffer> createEmpty() {
-  static auto cls = JByteBuffer::javaClassStatic();
-  static auto meth = cls->getStaticMethod<JByteBuffer::javaobject(int)>("allocateDirect");
-  return meth(cls, 0);
-}
-}
-
 void JBuffer::rewind() const {
   static auto meth = javaClassStatic()->getMethod<alias_ref<JBuffer>()>("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_t>(size);
+}
+
+bool JBuffer::isDirect() const {
+  static auto meth = javaClassStatic()->getMethod<jboolean()>("isDirect");
+  return meth(self());
+}
+
 local_ref<JByteBuffer> 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<javaobject>(Environment::current()->NewDirectByteBuffer(data, size)));
   FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
@@ -40,39 +67,10 @@ local_ref<JByteBuffer> JByteBuffer::wrapBytes(uint8_t* data, size_t size) {
   return res;
 }
 
-uint8_t* JByteBuffer::getDirectBytes() const {
+local_ref<JByteBuffer> JByteBuffer::allocateDirect(jint size) {
-  if (!self()) {
+  static auto cls = JByteBuffer::javaClassStatic();
-    throwNewJavaException("java/lang/NullPointerException", "java.lang.NullPointerException");
+  static auto meth = cls->getStaticMethod<JByteBuffer::javaobject(int)>("allocateDirect");
-  }
+  return meth(cls, size);
-  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<uint8_t*>(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_t>(size);
-}
-
-bool JByteBuffer::isDirect() const {
-  static auto meth = javaClassStatic()->getMethod<jboolean()>("isDirect");
-  return meth(self());
 }
 
 }}

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<JByteBuffer, JBuffer> {
   static constexpr const char* kJavaDescriptor = "Ljava/nio/ByteBuffer;";
 
   static local_ref<JByteBuffer> wrapBytes(uint8_t* data, size_t size);
+  static local_ref<JByteBuffer> allocateDirect(jint size);
 
-  bool isDirect() const;
+  uint8_t* getDirectBytes() const {
+    return static_cast<uint8_t*>(getDirectAddress());
+  }
 
-  uint8_t* getDirectBytes() const;
+  size_t getDirectSize() const {
-  size_t getDirectSize() const;
+    return getDirectCapacity();
+  }
 };
 
 }}

libs/fbjni/cxx/fbjni/detail/Boxed.h

@@ -56,6 +56,14 @@ DEFINE_BOXED_PRIMITIVE(double, Double)
 
 #undef DEFINE_BOXED_PRIMITIVE
 
+template<typename T>
+inline typename std::enable_if<
+  (std::is_same<T, long long>::value || std::is_same<T, int64_t>::value) && !std::is_same<T, jlong>::value,
+  local_ref<jobject>
+>::type autobox(T val) {
+  return JLong::valueOf(val);
+}
+
 struct JVoid : public jni::JavaClass<JVoid> {
   static auto constexpr kJavaDescriptor = "Ljava/lang/Void;";
 };

libs/fbjni/cxx/fbjni/detail/CoreClasses-inl.h

@@ -48,6 +48,11 @@ inline void JObject::setFieldValue(JField<T> field, T value) noexcept {
   field.set(self(), value);
 }
 
+template<typename T, typename>
+inline void JObject::setFieldValue(JField<T> field, alias_ref<T> value) noexcept {
+  setFieldValue(field, value.get());
+}
+
 inline std::string JObject::toString() const {
   static const auto method = findClassLocal("java/lang/Object")->getMethod<jstring()>("toString");
 
@@ -288,6 +293,11 @@ inline void JClass::setStaticFieldValue(JStaticField<T> field, T value) noexcept
   field.set(self(), value);
 }
 
+template<typename T, typename>
+inline void JClass::setStaticFieldValue(JStaticField<T> field, alias_ref<T> value) noexcept {
+  setStaticFieldValue(field, value.get());
+}
+
 template<typename R, typename... Args>
 inline local_ref<R> JClass::newObject(
     JConstructor<R(Args...)> constructor,
@@ -444,7 +454,6 @@ local_ref<typename JArrayClass<T>::javaobject> adopt_local_array(jobjectArray re
 template <typename JArrayType>
 auto JPrimitiveArray<JArrayType>::getRegion(jsize start, jsize length)
     -> std::unique_ptr<T[]> {
-  using T = typename jtype_traits<JArrayType>::entry_type;
   auto buf = std::unique_ptr<T[]>{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<T*>(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);
   }

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<JObject> lhs, alias_ref<JObject> 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<typename T>
   local_ref<T*> getFieldValue(JField<T*> field) const noexcept;
 
-  /// Set the value of field. Any Java type is accepted, including the primitive types
+  /// Set the value of field. Any Java type is accepted.
-  /// and raw reference types.
   template<typename T>
   void setFieldValue(JField<T> field, T value) noexcept;
+  template<typename T,
+           typename = typename std::enable_if<IsPlainJniReference<T>(), T>::type>
+  void setFieldValue(JField<T> field, alias_ref<T> value) noexcept;
 
   /// Convenience method to create a std::string representing the object
   std::string toString() const;
@@ -233,11 +242,18 @@ class JClass : public JavaClass<JClass, JObject, jclass> {
   ///     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<NativeMethod> methods);
 
   /// Check to see if the class is assignable from another class
@@ -287,10 +303,12 @@ class JClass : public JavaClass<JClass, JObject, jclass> {
   template<typename T>
   local_ref<T*> getStaticFieldValue(JStaticField<T*> field) noexcept;
 
-  /// Set the value of field. Any Java type is accepted, including the primitive types
+  /// Set the value of field. Any Java type is accepted.
-  /// and raw reference types.
   template<typename T>
   void setStaticFieldValue(JStaticField<T> field, T value) noexcept;
+  template<typename T,
+           typename = typename std::enable_if<IsPlainJniReference<T>(), T>::type>
+  void setStaticFieldValue(JStaticField<T> field, alias_ref<T> value) noexcept;
 
   /// Allocates a new object and invokes the specified constructor
   template<typename R, typename... Args>

libs/fbjni/cxx/fbjni/detail/Exceptions.h

@@ -64,7 +64,7 @@ class JCppException : public JavaClass<JCppException, JThrowable> {
 class JniException : public std::exception {
  public:
   JniException();
-  ~JniException();
+  ~JniException() override;
 
   explicit JniException(alias_ref<jthrowable> throwable);
 
@@ -74,7 +74,7 @@ class JniException : public std::exception {
 
   local_ref<JThrowable> getThrowable() const noexcept;
 
-  virtual const char* what() const noexcept;
+  const char* what() const noexcept override;
 
   void setJavaException() const noexcept;
 

libs/fbjni/cxx/fbjni/detail/Hybrid.h

@@ -31,7 +31,7 @@ class HybridDestructor : public JavaClass<HybridDestructor> {
   public:
     static auto constexpr kJavaDescriptor = "Lcom/facebook/jni/HybridData$Destructor;";
 
-  detail::BaseHybridClass* getNativePointer();
+  detail::BaseHybridClass* getNativePointer() const;
 
   void setNativePointer(std::unique_ptr<detail::BaseHybridClass> 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<Base>::CxxBase::CxxBase;
 
   static void registerHybrid(std::initializer_list<NativeMethod> methods) {
-    javaClassStatic()->registerNatives(methods);
+    javaClassLocal()->registerNatives(methods);
   }
 
   static local_ref<detail::HybridData> makeHybridData(std::unique_ptr<T> 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 <typename T, typename B>
-inline T* HybridClass<T, B>::JavaPart::cthis() {
+inline T* HybridClass<T, B>::JavaPart::cthis() const {
   detail::BaseHybridClass* result = 0;
   static bool isHybrid = detail::HybridClassBase::isHybridClassBase(this->getClass());
   if (isHybrid) {

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 <typename... Args>

libs/fbjni/cxx/fbjni/detail/MetaConvert.h

@@ -67,6 +67,25 @@ struct Convert<bool> {
   }
 };
 
+// Sometimes (64-bit Android) jlong is "long long", but int64_t is "long".
+// Allow int64_t to work as jlong.
+template<typename T>
+struct Convert<T,
+    typename std::enable_if<
+      (std::is_same<T, long long>::value || std::is_same<T, int64_t>::value) && !std::is_same<T, jlong>::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<T> from T
 template <typename T>
 struct Convert<alias_ref<T>> {

libs/fbjni/cxx/fbjni/detail/References-inl.h

@@ -174,6 +174,29 @@ operator!=(const T1& a, const T2& b) {
   return !(a == b);
 }
 
+template<typename T1>
+inline enable_if_t<IsNonWeakReference<T1>(), bool>
+operator==(const T1& a, std::nullptr_t) {
+  return getPlainJniReference(a) == nullptr;
+}
+
+template<typename T1>
+inline enable_if_t<IsNonWeakReference<T1>(), bool>
+operator==(std::nullptr_t, const T1& a) {
+  return nullptr == getPlainJniReference(a);
+}
+
+template<typename T1>
+inline enable_if_t<IsNonWeakReference<T1>(), bool>
+operator!=(const T1& a, std::nullptr_t) {
+  return !(a == nullptr);
+}
+
+template<typename T1>
+inline enable_if_t<IsNonWeakReference<T1>(), bool>
+operator!=(std::nullptr_t, const T1& a) {
+  return !(nullptr == getPlainJniReference(a));
+}
 
 // base_owned_ref ///////////////////////////////////////////////////////////////////////
 
@@ -185,7 +208,9 @@ inline base_owned_ref<T, Alloc>::base_owned_ref() noexcept
 template<typename T, typename Alloc>
 inline base_owned_ref<T, Alloc>::base_owned_ref(std::nullptr_t t) noexcept
   : base_owned_ref(static_cast<javaobject>(nullptr))
-{}
+{
+  (void)t;
+}
 
 template<typename T, typename Alloc>
 inline base_owned_ref<T, Alloc>::base_owned_ref(const base_owned_ref& other)

libs/fbjni/cxx/fbjni/detail/References.cpp

@@ -29,12 +29,23 @@ namespace {
 #ifdef __ANDROID__
 
 int32_t getAndroidApiLevel() {
-  auto cls = findClassLocal("android/os/Build$VERSION");
+  // This is called from the static local initializer in
-  auto fld = cls->getStaticField<int32_t>("SDK_INT");
+  // isObjectRefType(), and creating fbjni references can call
-  if (fld) {
+  // isObjectRefType().  So, to avoid recursively entering the block
-    return cls->getStaticFieldValue(fld);
+  // 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<jint>::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
+  static bool getObjectRefTypeWorks = doesGetObjectRefTypeWork();
-// causes sample app to freeze
-  static bool getObjectRefTypeWorks = true;
 
   return
-    !reference ||
     !getObjectRefTypeWorks ||
     Environment::current()->GetObjectRefType(reference) == refType;
 }

libs/fbjni/cxx/fbjni/detail/References.h

@@ -249,6 +249,26 @@ template<typename T1, typename T2>
 enable_if_t<IsNonWeakReference<T1>() && IsNonWeakReference<T2>(), bool>
 operator!=(const T1& a, const T2& b);
 
+/**
+ * Compare references against nullptr
+ */
+template<typename T1>
+enable_if_t<IsNonWeakReference<T1>(), bool>
+operator==(const T1& a, std::nullptr_t);
+
+template<typename T1>
+enable_if_t<IsNonWeakReference<T1>(), bool>
+operator==(std::nullptr_t, const T1& a);
+
+template<typename T1>
+enable_if_t<IsNonWeakReference<T1>(), bool>
+operator!=(const T1& a, std::nullptr_t);
+
+template<typename T1>
+enable_if_t<IsNonWeakReference<T1>(), bool>
+operator!=(std::nullptr_t, const T1& a);
+
+
 template<typename T, typename Alloc>
 class base_owned_ref {
  public:

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<R(Args...)>::descriptor();
 }
 
+template<typename R, typename ...Args>
+template<R(*func)(Args...)>
+JNI_ENTRY_POINT R CriticalMethod<R(*)(Args...)>::call(alias_ref<jclass>, Args... args) noexcept {
+  static_assert(
+    IsJniPrimitive<R>() || std::is_void<R>(),
+    "Critical Native Methods may only return primitive JNI types, or void.");
+  static_assert(
+    AreJniPrimitives<Args...>(),
+    "Critical Native Methods may only use primitive JNI types as parameters");
+
+  return func(std::forward<Args>(args)...);
+}
+
+template<typename R, typename ...Args>
+template<R(*func)(Args...)>
+inline std::string CriticalMethod<R(*)(Args...)>::desc() {
+  return makeDescriptor(call<func>);
+}
+
 }
 
 }}

libs/fbjni/cxx/fbjni/detail/Registration.h

@@ -44,6 +44,18 @@ std::string makeDescriptor(R (*func)(alias_ref<C>, Args... args));
 template<typename R, typename C, typename... Args>
 std::string makeDescriptor(R (C::*method0)(Args... args));
 
+template<typename F>
+struct CriticalMethod;
+
+template<typename R, typename ...Args>
+struct CriticalMethod<R(*)(Args...)> {
+  template<R(*func)(Args...)>
+  static R call(alias_ref<jclass>, Args... args) noexcept;
+
+  template<R(*func)(Args...)>
+  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<decltype(&func)>::call<&func>)
+
+#define makeCriticalNativeMethod2(name, func)                                \
+  makeCriticalNativeMethod3(                                                 \
+    name,                                                                    \
+    ::facebook::jni::detail::CriticalMethod<decltype(&func)>::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"

libs/fbjni/cxx/fbjni/detail/TypeTraits.h

@@ -69,6 +69,25 @@ constexpr bool IsJniPrimitive() {
   return is_jni_primitive<T>::value;
 }
 
+/// Metafunction to determine whether a series of types are all primitive JNI types.
+template<typename ...Ts>
+struct are_jni_primitives;
+
+template<typename T, typename ...Ts>
+struct are_jni_primitives<T, Ts...> :
+  std::integral_constant<bool,
+    is_jni_primitive<T>::value && are_jni_primitives<Ts...>::value> {};
+
+template<>
+struct are_jni_primitives<> : std::integral_constant<bool, true> {};
+
+/// Helper to simplify use of are_jni_primitives
+template<typename ...Ts>
+constexpr bool AreJniPrimitives() {
+  return are_jni_primitives<Ts...>::value;
+}
+
+
 /// Metafunction to determine whether a type is a JNI array of primitives or not
 template <typename T>
 struct is_jni_primitive_array :

libs/fbjni/cxx/fbjni/fbjni.cpp

@@ -52,26 +52,29 @@ jint initialize(JavaVM* vm, std::function<void()>&& init_fn) noexcept {
   return JNI_VERSION_1_6;
 }
 
-alias_ref<JClass> findClassStatic(const char* name) {
+namespace detail {
-  const auto env = detail::currentOrNull();
+
+jclass findClass(JNIEnv* env, const char* name) {
   if (!env) {
     throw std::runtime_error("Unable to retrieve JNIEnv*.");
   }
-  local_ref<jclass> 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());
+  return cls;
-  FACEBOOK_JNI_THROW_EXCEPTION_IF(!leaking_ref);
+}
-  return wrap_alias(leaking_ref);
+
 }
 
 local_ref<JClass> findClassLocal(const char* name) {
-  const auto env = detail::currentOrNull();
+  return adopt_local(detail::findClass(detail::currentOrNull(), name));
-  if (!env) {
-    throw std::runtime_error("Unable to retrieve JNIEnv*.");
 }
-  auto cls = env->FindClass(name);
+
-  FACEBOOK_JNI_THROW_EXCEPTION_IF(!cls);
+alias_ref<JClass> findClassStatic(const char* name) {
-  return adopt_local(cls);
+  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<JString> 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<jlong>("mNativePointer");
   auto* value = reinterpret_cast<detail::BaseHybridClass*>(getFieldValue(pointerField));
   if (!value) {

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 final ReferenceQueue sReferenceQueue;
-  private static Thread sThread;
+  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<Destructor> mHead = new AtomicReference<>();
+    private final AtomicReference<Destructor> 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();

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.