Android: change the way JNIEnv is retrieved
authorSylvain Becker <sylvain.becker@gmail.com>
Fri, 11 Jan 2019 21:42:52 +0100
changeset 1252986c22cfe2d7d
parent 12528 e3fddfb03620
child 12530 63abc8d1ca3a
Android: change the way JNIEnv is retrieved

- Currently, it tries to Attach the JVM first and update the thread local storage, which are two operations.
Now, it simply gives back the JNI Env stored for the thread.

- Android_JNI_SetupThreadi() should only be used for external.
For internal SDL thread, it's already called in RunThread() (SDL_systhread.c),
and other thread are Java threads which don't need to be attached. i
(even if it doesn't hurt to do it, since it's a no-op).

- JNI_OnLoad is filled with pthread_create, GetEnv, AttachCurrentThread...
It's called for all shared libraries which may don't want this setup,
and loading libraries can be also modified to be done from a static context,
or with relinker. So it's not really clear how, who and what it sets up.
=> Reduce this function to the minimal
src/core/android/SDL_android.c
     1.1 --- a/src/core/android/SDL_android.c	Fri Jan 11 15:36:16 2019 +0100
     1.2 +++ b/src/core/android/SDL_android.c	Fri Jan 11 21:42:52 2019 +0100
     1.3 @@ -294,7 +294,7 @@
     1.4  static SDL_bool bHasEnvironmentVariables = SDL_FALSE;
     1.5  
     1.6  
     1.7 -static void Android_JNI_SetEnv(JNIEnv *env);
     1.8 +static int Android_JNI_SetEnv(JNIEnv *env);
     1.9  
    1.10  /*******************************************************************************
    1.11                   Functions called by JNI
    1.12 @@ -321,21 +321,7 @@
    1.13  /* Library init */
    1.14  JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *reserved)
    1.15  {
    1.16 -    JNIEnv *env;
    1.17      mJavaVM = vm;
    1.18 -    LOGI("JNI_OnLoad called");
    1.19 -    if ((*mJavaVM)->GetEnv(mJavaVM, (void **) &env, JNI_VERSION_1_4) != JNI_OK) {
    1.20 -        LOGE("Failed to get the environment using GetEnv()");
    1.21 -        return -1;
    1.22 -    }
    1.23 -    /*
    1.24 -     * Create mThreadKey so we can keep track of the JNIEnv assigned to each thread
    1.25 -     * Refer to http://developer.android.com/guide/practices/design/jni.html for the rationale behind this
    1.26 -     */
    1.27 -    Android_JNI_CreateKey_once();
    1.28 -    
    1.29 -    Android_JNI_SetupThread();
    1.30 -
    1.31      return JNI_VERSION_1_4;
    1.32  }
    1.33  
    1.34 @@ -354,6 +340,19 @@
    1.35  {
    1.36      __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "nativeSetupJNI()");
    1.37  
    1.38 +    /*
    1.39 +     * Create mThreadKey so we can keep track of the JNIEnv assigned to each thread
    1.40 +     * Refer to http://developer.android.com/guide/practices/design/jni.html for the rationale behind this
    1.41 +     */
    1.42 +    Android_JNI_CreateKey_once();
    1.43 +
    1.44 +    /* Save JNIEnv of SDLActivity */
    1.45 +    Android_JNI_SetEnv(env);
    1.46 +
    1.47 +    if (mJavaVM == NULL) {
    1.48 +        __android_log_print(ANDROID_LOG_ERROR, "SDL", "failed to found a JavaVM");
    1.49 +    }
    1.50 +
    1.51      /* Use a mutex to prevent concurrency issues between Java Activity and Native thread code, when using 'Android_Window'.
    1.52       * (Eg. Java sending Touch events, while native code is destroying the main SDL_Window. )
    1.53       */
    1.54 @@ -365,8 +364,6 @@
    1.55          __android_log_print(ANDROID_LOG_ERROR, "SDL", "failed to create Android_ActivityMutex mutex");
    1.56      }
    1.57  
    1.58 -    Android_JNI_SetupThread();
    1.59 -
    1.60      mActivityClass = (jclass)((*env)->NewGlobalRef(env, cls));
    1.61  
    1.62      midGetNativeSurface = (*env)->GetStaticMethodID(env, mActivityClass,
    1.63 @@ -444,8 +441,6 @@
    1.64  {
    1.65      __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "AUDIO nativeSetupJNI()");
    1.66  
    1.67 -    Android_JNI_SetupThread();
    1.68 -
    1.69      mAudioManagerClass = (jclass)((*env)->NewGlobalRef(env, cls));
    1.70  
    1.71      midAudioOpen = (*env)->GetStaticMethodID(env, mAudioManagerClass,
    1.72 @@ -484,8 +479,6 @@
    1.73  {
    1.74      __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "CONTROLLER nativeSetupJNI()");
    1.75  
    1.76 -    Android_JNI_SetupThread();
    1.77 -
    1.78      mControllerManagerClass = (jclass)((*env)->NewGlobalRef(env, cls));
    1.79  
    1.80      midPollInputDevices = (*env)->GetStaticMethodID(env, mControllerManagerClass,
    1.81 @@ -516,6 +509,9 @@
    1.82  
    1.83      __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "nativeRunMain()");
    1.84  
    1.85 +    /* Save JNIEnv of SDLThread */
    1.86 +    Android_JNI_SetEnv(env);
    1.87 +
    1.88      library_file = (*env)->GetStringUTFChars(env, library, NULL);
    1.89      library_handle = dlopen(library_file, RTLD_GLOBAL);
    1.90      if (library_handle) {
    1.91 @@ -1155,11 +1151,12 @@
    1.92      }
    1.93  }
    1.94  
    1.95 -static void Android_JNI_SetEnv(JNIEnv *env) {
    1.96 +static int Android_JNI_SetEnv(JNIEnv *env) {
    1.97      int status = pthread_setspecific(mThreadKey, env);
    1.98      if (status < 0) {
    1.99          __android_log_print(ANDROID_LOG_ERROR, "SDL", "Failed pthread_setspecific() in Android_JNI_SetEnv() (err=%d)", status);
   1.100      }
   1.101 +    return status;
   1.102  }
   1.103  
   1.104  JNIEnv* Android_JNI_GetEnv(void)
   1.105 @@ -1176,13 +1173,6 @@
   1.106       * Note: You can call this function any number of times for the same thread, there's no harm in it
   1.107       */
   1.108  
   1.109 -    JNIEnv *env;
   1.110 -    int status = (*mJavaVM)->AttachCurrentThread(mJavaVM, &env, NULL);
   1.111 -    if (status < 0) {
   1.112 -        LOGE("failed to attach current thread");
   1.113 -        return 0;
   1.114 -    }
   1.115 -
   1.116      /* From http://developer.android.com/guide/practices/jni.html
   1.117       * Threads attached through JNI must call DetachCurrentThread before they exit. If coding this directly is awkward,
   1.118       * in Android 2.0 (Eclair) and higher you can use pthread_key_create to define a destructor function that will be
   1.119 @@ -1192,14 +1182,43 @@
   1.120       * Note: You can call this function any number of times for the same thread, there's no harm in it
   1.121       *       (except for some lost CPU cycles)
   1.122       */
   1.123 -    Android_JNI_SetEnv(env);
   1.124 +
   1.125 +
   1.126 +
   1.127 +    /* Get JNIEnv from the Thread local storage */
   1.128 +    JNIEnv *env = pthread_getspecific(mThreadKey);
   1.129 +    if (env == NULL) {
   1.130 +        __android_log_print(ANDROID_LOG_ERROR, "SDL", "JNIEnv is NULL. Call Android_JNI_SetupThread() first.");
   1.131 +    }
   1.132  
   1.133      return env;
   1.134  }
   1.135  
   1.136  int Android_JNI_SetupThread(void)
   1.137  {
   1.138 -    Android_JNI_GetEnv();
   1.139 +    JNIEnv *env;
   1.140 +    int status;
   1.141 +
   1.142 +    /* There should be a JVM */
   1.143 +    if (mJavaVM == NULL) {
   1.144 +        __android_log_print(ANDROID_LOG_ERROR, "SDL", "Failed, there is no JavaVM");
   1.145 +        return 0;
   1.146 +    }
   1.147 +
   1.148 +    /* Attach the current thread to the JVM and get a JNIEnv.
   1.149 +     * It will be detached by pthread_create destructor 'Android_JNI_ThreadDestroyed'
   1.150 +     */
   1.151 +    status = (*mJavaVM)->AttachCurrentThread(mJavaVM, &env, NULL);
   1.152 +    if (status < 0) {
   1.153 +        __android_log_print(ANDROID_LOG_ERROR, "SDL", "Failed to attach current thread (err=%d)", status);
   1.154 +        return 0;
   1.155 +    }
   1.156 +
   1.157 +    /* Save JNIEnv into the Thread local storage */
   1.158 +    if (Android_JNI_SetEnv(env) < 0) {
   1.159 +        return 0;
   1.160 +    }
   1.161 +
   1.162      return 1;
   1.163  }
   1.164