[vlc-devel] [PATCH] vout/android: utils: don't cache JNIEnv

[Zhao Zhili]

Sometimes android_getEnvCommon() is called in a thread created by the
libvlc user. If the user called DetachCurrentThread first (for example,
use non-optimized RAII strategy to AttachCurrentThread/DetachCurrentThread
each time), then the cached JNIEnv become invalid.

(*jvm)->GetEnv is about 3 times slower than pthread_getspecific in my
test case. Since android_getEnv isn't called in a inner loop, it's not a
bottleneck.
---
 modules/video_output/android/utils.c | 66 ++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 37 deletions(-)

diff --git a/modules/video_output/android/utils.c b/modules/video_output/android/utils.c
index de65746..24df7b6 100644
--- a/modules/video_output/android/utils.c
+++ b/modules/video_output/android/utils.c
@@ -299,27 +299,23 @@ LoadNativeWindowAPI(AWindowHandler *p_awh)
  * Andoid JNIEnv helper
  */
 
-static pthread_key_t jni_env_key;
-static pthread_once_t jni_env_key_once = PTHREAD_ONCE_INIT;
+static pthread_key_t jni_jvm_key;
+static pthread_once_t jni_jvm_key_once = PTHREAD_ONCE_INIT;
 
 /* This function is called when a thread attached to the Java VM is canceled or
  * exited */
 static void
 jni_detach_thread(void *data)
 {
-    JNIEnv *env = data;
-    JavaVM *jvm;
-
-    (*env)->GetJavaVM(env, &jvm);
-    assert(jvm);
+    JavaVM *jvm = data;
     (*jvm)->DetachCurrentThread(jvm);
 }
 
-static void jni_env_key_create()
+static void jni_jvm_key_create()
 {
     /* Create a TSD area and setup a destroy callback when a thread that
-     * previously set the jni_env_key is canceled or exited */
-    pthread_key_create(&jni_env_key, jni_detach_thread);
+     * previously set the jni_jvm_key is canceled or exited */
+    pthread_key_create(&jni_jvm_key, jni_detach_thread);
 }
 
 static JNIEnv *
@@ -327,39 +323,35 @@ android_getEnvCommon(vlc_object_t *p_obj, JavaVM *jvm, const char *psz_name)
 {
     assert((p_obj && !jvm) || (!p_obj && jvm));
 
-    JNIEnv *env;
-
-    pthread_once(&jni_env_key_once, jni_env_key_create);
-    env = pthread_getspecific(jni_env_key);
-    if (env == NULL)
+    if (!jvm)
     {
+        jvm = var_InheritAddress(p_obj, "android-jvm");
         if (!jvm)
-            jvm = var_InheritAddress(p_obj, "android-jvm");
+            return NULL;
+    }
 
-        if (!jvm)
+    JNIEnv *env = NULL;
+    pthread_once(&jni_jvm_key_once, jni_jvm_key_create);
+    /* if GetEnv returns JNI_OK, the thread is already attached to the
+     * JavaVM, so we are already in a java thread, and we don't have to
+     * setup any destroy callbacks */
+    if ((*jvm)->GetEnv(jvm, (void **)&env, JNI_VERSION_1_2) != JNI_OK)
+    {
+        /* attach the thread to the Java VM */
+        JavaVMAttachArgs args;
+
+        args.version = JNI_VERSION_1_2;
+        args.name = psz_name;
+        args.group = NULL;
+
+        if ((*jvm)->AttachCurrentThread(jvm, &env, &args) != JNI_OK)
             return NULL;
 
-        /* if GetEnv returns JNI_OK, the thread is already attached to the
-         * JavaVM, so we are already in a java thread, and we don't have to
-         * setup any destroy callbacks */
-        if ((*jvm)->GetEnv(jvm, (void **)&env, JNI_VERSION_1_2) != JNI_OK)
+        /* save jvm which will be used to detach current thread */
+        if (pthread_setspecific(jni_jvm_key, jvm) != 0)
         {
-            /* attach the thread to the Java VM */
-            JavaVMAttachArgs args;
-
-            args.version = JNI_VERSION_1_2;
-            args.name = psz_name;
-            args.group = NULL;
-
-            if ((*jvm)->AttachCurrentThread(jvm, &env, &args) != JNI_OK)
-                return NULL;
-
-            /* Set the attached env to the thread-specific data area (TSD) */
-            if (pthread_setspecific(jni_env_key, env) != 0)
-            {
-                (*jvm)->DetachCurrentThread(jvm);
-                return NULL;
-            }
+            (*jvm)->DetachCurrentThread(jvm);
+            return NULL;
         }
     }
 
-- 
2.9.5