[jni] Add a call to jvm.DetachCurrentThread when the thread detaches

pkgs/jni/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 0.9.3-wip
+
+- Added lifetime event handling for the thread-local JNI env. Now
+  `jvm.DetachNativeThread` is called when the thread detaches as recommended
+  [here](https://developer.android.com/training/articles/perf-jni#threads).
+
 ## 0.9.2
 
 - Bumped `minSdk` to 21.

pkgs/jni/pubspec.yaml

@@ -4,7 +4,7 @@
 
 name: jni
 description: A library to access JNI from Dart and Flutter that acts as a support library for package:jnigen.
-version: 0.9.2
+version: 0.9.3-wip
 repository: https://github.com/dart-lang/native/tree/main/pkgs/jni
 
 topics:

pkgs/jni/src/dartjni.c

@@ -8,9 +8,9 @@
 
 #include "dartjni.h"
 
-void initAllLocks(JniLocks* locks) {
-  init_lock(&locks->classLoadingLock);
-}
+#ifndef _WIN32
+pthread_key_t tlsKey;
+#endif
 
 /// Load class through platform-specific mechanism.
 ///
@@ -44,27 +44,59 @@ JniContext jni_context = {
 };
 
 JniContext* jni = &jni_context;
+THREAD_LOCAL JNIEnv* jniEnv = NULL;
+JniExceptionMethods exceptionMethods;
 
-thread_local JNIEnv* jniEnv = NULL;
+void init() {
+#ifndef _WIN32
+  // Init TLS keys.
+  pthread_key_create(&tlsKey, detach_thread);
+#endif
 
-JniExceptionMethods exceptionMethods;
+  // Init locks.
+  init_lock(&jni_context.locks.classLoadingLock);
 
-void initExceptionHandling(JniExceptionMethods* methods) {
-  methods->objectClass = FindClass("java/lang/Object");
-  methods->exceptionClass = FindClass("java/lang/Exception");
-  methods->printStreamClass = FindClass("java/io/PrintStream");
-  methods->byteArrayOutputStreamClass =
+  // Init exception handling classes and methods.
+  exceptionMethods.objectClass = FindClass("java/lang/Object");
+  exceptionMethods.exceptionClass = FindClass("java/lang/Exception");
+  exceptionMethods.printStreamClass = FindClass("java/io/PrintStream");
+  exceptionMethods.byteArrayOutputStreamClass =
       FindClass("java/io/ByteArrayOutputStream");
-  load_method(methods->objectClass, &methods->toStringMethod, "toString",
-              "()Ljava/lang/String;");
-  load_method(methods->exceptionClass, &methods->printStackTraceMethod,
-              "printStackTrace", "(Ljava/io/PrintStream;)V");
-  load_method(methods->byteArrayOutputStreamClass,
-              &methods->byteArrayOutputStreamCtor, "<init>", "()V");
-  load_method(methods->printStreamClass, &methods->printStreamCtor, "<init>",
+  load_method(exceptionMethods.objectClass, &exceptionMethods.toStringMethod,
+              "toString", "()Ljava/lang/String;");
+  load_method(exceptionMethods.exceptionClass,
+              &exceptionMethods.printStackTraceMethod, "printStackTrace",
+              "(Ljava/io/PrintStream;)V");
+  load_method(exceptionMethods.byteArrayOutputStreamClass,
+              &exceptionMethods.byteArrayOutputStreamCtor, "<init>", "()V");
+  load_method(exceptionMethods.printStreamClass,
+              &exceptionMethods.printStreamCtor, "<init>",
               "(Ljava/io/OutputStream;)V");
 }
 
+void deinit() {
+#ifndef _WIN32
+  // Delete TLS keys.
+  pthread_key_delete(tlsKey);
+#endif
+
+  // Destroy locks.
+  destroy_lock(&jni_context.locks.classLoadingLock);
+
+  // Delete references to exception handling classes.
+  if (jniEnv != NULL) {
+    (*jniEnv)->DeleteGlobalRef(jniEnv, exceptionMethods.objectClass);
+    (*jniEnv)->DeleteGlobalRef(jniEnv, exceptionMethods.exceptionClass);
+    (*jniEnv)->DeleteGlobalRef(jniEnv, exceptionMethods.printStreamClass);
+    (*jniEnv)->DeleteGlobalRef(jniEnv,
+                               exceptionMethods.byteArrayOutputStreamClass);
+  }
+}
+
+JNIEXPORT void JNICALL JNI_OnUnload(JavaVM* jvm, void* reserved) {
+  deinit();
+}
+
 /// Get JVM associated with current process.
 /// Returns NULL if no JVM is running.
 FFI_PLUGIN_EXPORT
@@ -108,8 +140,7 @@ Java_com_github_dart_1lang_jni_JniPlugin_initializeJni(JNIEnv* env,
   jni_context.loadClassMethod =
       (*env)->GetMethodID(env, classLoaderClass, "loadClass",
                           "(Ljava/lang/String;)Ljava/lang/Class;");
-  initAllLocks(&jni_context.locks);
-  initExceptionHandling(&exceptionMethods);
+  init();
 }
 
 JNIEXPORT void JNICALL
@@ -146,6 +177,11 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL,   // handle to DLL module
       // Return FALSE to fail DLL load.
       InitializeCriticalSection(&spawnLock);
       break;
+    case DLL_THREAD_DETACH:
+      if (jniEnv != NULL) {
+        detach_thread(jniEnv);
+      }
+      break;
     case DLL_PROCESS_DETACH:
       // Perform any necessary cleanup.
       DeleteCriticalSection(&spawnLock);
@@ -184,8 +220,7 @@ int SpawnJvm(JavaVMInitArgs* initArgs) {
   if (flag != JNI_OK) {
     return flag;
   }
-  initAllLocks(&jni_context.locks);
-  initExceptionHandling(&exceptionMethods);
+  init();
   release_lock(&spawnLock);
 
   return JNI_OK;
@@ -771,6 +806,7 @@ Dart_FinalizableHandle newJObjectFinalizableHandle(Dart_Handle object,
       return Dart_NewFinalizableHandle_DL(object, reference, 0,
                                           finalizeWeakGlobal);
   }
+  return NULL;  // Never happens.
 }
 
 FFI_PLUGIN_EXPORT
@@ -782,7 +818,7 @@ Dart_FinalizableHandle newBooleanFinalizableHandle(Dart_Handle object,
 FFI_PLUGIN_EXPORT
 void deleteFinalizableHandle(Dart_FinalizableHandle finalizableHandle,
                              Dart_Handle object) {
-  return Dart_DeleteFinalizableHandle_DL(finalizableHandle, object);
+  Dart_DeleteFinalizableHandle_DL(finalizableHandle, object);
 }
 
 jclass _c_Object = NULL;

pkgs/jni/src/dartjni.h

@@ -12,23 +12,23 @@
 #include <stdio.h>
 #include <stdlib.h>
 
-#if _WIN32
+#ifdef _WIN32
 #include <windows.h>
 #else
 #include <pthread.h>
 #include <unistd.h>
 #endif
 
-#if _WIN32
+#ifdef _WIN32
 #define FFI_PLUGIN_EXPORT __declspec(dllexport)
 #else
 #define FFI_PLUGIN_EXPORT
 #endif
 
-#if defined _WIN32
-#define thread_local __declspec(thread)
+#ifdef _WIN32
+#define THREAD_LOCAL __declspec(thread)
 #else
-#define thread_local __thread
+#define THREAD_LOCAL __thread
 #endif
 
 #ifdef __ANDROID__
@@ -43,7 +43,7 @@
 
 /// Locking functions for windows and pthread.
 
-#if defined _WIN32
+#ifdef _WIN32
 #include <windows.h>
 
 typedef CRITICAL_SECTION MutexLock;
@@ -85,8 +85,7 @@ static inline void free_mem(void* mem) {
   CoTaskMemFree(mem);
 }
 
-#elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ ||         \
-    defined __GNUC__
+#else
 #include <pthread.h>
 
 typedef pthread_mutex_t MutexLock;
@@ -127,11 +126,6 @@ static inline void destroy_cond(ConditionVariable* cond) {
 static inline void free_mem(void* mem) {
   free(mem);
 }
-
-#else
-
-#error "No locking/condition variable support; Possibly unsupported platform"
-
 #endif
 
 typedef struct CallbackResult {
@@ -160,10 +154,32 @@ typedef struct JniContext {
 
 // jniEnv for this thread, used by inline functions in this header,
 // therefore declared as extern.
-extern thread_local JNIEnv* jniEnv;
+extern THREAD_LOCAL JNIEnv* jniEnv;
 
 extern JniContext* jni;
 
+/// Handling the lifetime of thread-local jniEnv.
+#ifndef _WIN32
+extern pthread_key_t tlsKey;
+#endif
+
+static inline void detach_thread(void* data) {
+  jniEnv = NULL;
+
+  if (*jni->jvm) {
+    (*jni->jvm)->DetachCurrentThread(jni->jvm);
+  }
+}
+
+static inline void attach_thread() {
+  if (jniEnv == NULL) {
+    (*jni->jvm)->AttachCurrentThread(jni->jvm, __ENVP_CAST & jniEnv, NULL);
+#ifndef _WIN32
+    pthread_setspecific(tlsKey, &jniEnv);
+#endif
+  }
+}
+
 /// Types used by JNI API to distinguish between primitive types.
 enum JniType {
   booleanType = 0,
@@ -278,12 +294,6 @@ FFI_PLUGIN_EXPORT jobject GetApplicationContext(void);
 /// Returns current activity of the app on Android.
 FFI_PLUGIN_EXPORT jobject GetCurrentActivity(void);
 
-static inline void attach_thread() {
-  if (jniEnv == NULL) {
-    (*jni->jvm)->AttachCurrentThread(jni->jvm, __ENVP_CAST & jniEnv, NULL);
-  }
-}
-
 /// Load class into [cls] using platform specific mechanism
 static inline void load_class_platform(jclass* cls, const char* name) {
 #ifdef __ANDROID__