From 7f3478305fc4f8d31bb9a4184e3515a7aeb3af86 Mon Sep 17 00:00:00 2001 From: Sylvain Becker Date: Fri, 11 Jan 2019 21:42:52 +0100 Subject: [PATCH] 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 | 81 +++++++++++++++++++++------------- 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/src/core/android/SDL_android.c b/src/core/android/SDL_android.c index 40199fe6c52ec..38bb3df05149b 100644 --- a/src/core/android/SDL_android.c +++ b/src/core/android/SDL_android.c @@ -294,7 +294,7 @@ static SDL_bool bHasNewData; static SDL_bool bHasEnvironmentVariables = SDL_FALSE; -static void Android_JNI_SetEnv(JNIEnv *env); +static int Android_JNI_SetEnv(JNIEnv *env); /******************************************************************************* Functions called by JNI @@ -321,21 +321,7 @@ Android_JNI_CreateKey_once() /* Library init */ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *reserved) { - JNIEnv *env; mJavaVM = vm; - LOGI("JNI_OnLoad called"); - if ((*mJavaVM)->GetEnv(mJavaVM, (void **) &env, JNI_VERSION_1_4) != JNI_OK) { - LOGE("Failed to get the environment using GetEnv()"); - return -1; - } - /* - * Create mThreadKey so we can keep track of the JNIEnv assigned to each thread - * Refer to http://developer.android.com/guide/practices/design/jni.html for the rationale behind this - */ - Android_JNI_CreateKey_once(); - - Android_JNI_SetupThread(); - return JNI_VERSION_1_4; } @@ -354,6 +340,19 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(nativeSetupJNI)(JNIEnv *env, jclass cl { __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "nativeSetupJNI()"); + /* + * Create mThreadKey so we can keep track of the JNIEnv assigned to each thread + * Refer to http://developer.android.com/guide/practices/design/jni.html for the rationale behind this + */ + Android_JNI_CreateKey_once(); + + /* Save JNIEnv of SDLActivity */ + Android_JNI_SetEnv(env); + + if (mJavaVM == NULL) { + __android_log_print(ANDROID_LOG_ERROR, "SDL", "failed to found a JavaVM"); + } + /* Use a mutex to prevent concurrency issues between Java Activity and Native thread code, when using 'Android_Window'. * (Eg. Java sending Touch events, while native code is destroying the main SDL_Window. ) */ @@ -365,8 +364,6 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(nativeSetupJNI)(JNIEnv *env, jclass cl __android_log_print(ANDROID_LOG_ERROR, "SDL", "failed to create Android_ActivityMutex mutex"); } - Android_JNI_SetupThread(); - mActivityClass = (jclass)((*env)->NewGlobalRef(env, cls)); midGetNativeSurface = (*env)->GetStaticMethodID(env, mActivityClass, @@ -444,8 +441,6 @@ JNIEXPORT void JNICALL SDL_JAVA_AUDIO_INTERFACE(nativeSetupJNI)(JNIEnv *env, jcl { __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "AUDIO nativeSetupJNI()"); - Android_JNI_SetupThread(); - mAudioManagerClass = (jclass)((*env)->NewGlobalRef(env, cls)); midAudioOpen = (*env)->GetStaticMethodID(env, mAudioManagerClass, @@ -484,8 +479,6 @@ JNIEXPORT void JNICALL SDL_JAVA_CONTROLLER_INTERFACE(nativeSetupJNI)(JNIEnv *env { __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "CONTROLLER nativeSetupJNI()"); - Android_JNI_SetupThread(); - mControllerManagerClass = (jclass)((*env)->NewGlobalRef(env, cls)); midPollInputDevices = (*env)->GetStaticMethodID(env, mControllerManagerClass, @@ -516,6 +509,9 @@ JNIEXPORT int JNICALL SDL_JAVA_INTERFACE(nativeRunMain)(JNIEnv *env, jclass cls, __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "nativeRunMain()"); + /* Save JNIEnv of SDLThread */ + Android_JNI_SetEnv(env); + library_file = (*env)->GetStringUTFChars(env, library, NULL); library_handle = dlopen(library_file, RTLD_GLOBAL); if (library_handle) { @@ -1155,11 +1151,12 @@ static void Android_JNI_ThreadDestroyed(void *value) } } -static void Android_JNI_SetEnv(JNIEnv *env) { +static int Android_JNI_SetEnv(JNIEnv *env) { int status = pthread_setspecific(mThreadKey, env); if (status < 0) { __android_log_print(ANDROID_LOG_ERROR, "SDL", "Failed pthread_setspecific() in Android_JNI_SetEnv() (err=%d)", status); } + return status; } JNIEnv* Android_JNI_GetEnv(void) @@ -1176,13 +1173,6 @@ JNIEnv* Android_JNI_GetEnv(void) * Note: You can call this function any number of times for the same thread, there's no harm in it */ - JNIEnv *env; - int status = (*mJavaVM)->AttachCurrentThread(mJavaVM, &env, NULL); - if (status < 0) { - LOGE("failed to attach current thread"); - return 0; - } - /* From http://developer.android.com/guide/practices/jni.html * Threads attached through JNI must call DetachCurrentThread before they exit. If coding this directly is awkward, * in Android 2.0 (Eclair) and higher you can use pthread_key_create to define a destructor function that will be @@ -1192,14 +1182,43 @@ JNIEnv* Android_JNI_GetEnv(void) * Note: You can call this function any number of times for the same thread, there's no harm in it * (except for some lost CPU cycles) */ - Android_JNI_SetEnv(env); + + + + /* Get JNIEnv from the Thread local storage */ + JNIEnv *env = pthread_getspecific(mThreadKey); + if (env == NULL) { + __android_log_print(ANDROID_LOG_ERROR, "SDL", "JNIEnv is NULL. Call Android_JNI_SetupThread() first."); + } return env; } int Android_JNI_SetupThread(void) { - Android_JNI_GetEnv(); + JNIEnv *env; + int status; + + /* There should be a JVM */ + if (mJavaVM == NULL) { + __android_log_print(ANDROID_LOG_ERROR, "SDL", "Failed, there is no JavaVM"); + return 0; + } + + /* Attach the current thread to the JVM and get a JNIEnv. + * It will be detached by pthread_create destructor 'Android_JNI_ThreadDestroyed' + */ + status = (*mJavaVM)->AttachCurrentThread(mJavaVM, &env, NULL); + if (status < 0) { + __android_log_print(ANDROID_LOG_ERROR, "SDL", "Failed to attach current thread (err=%d)", status); + return 0; + } + + /* Save JNIEnv into the Thread local storage */ + if (Android_JNI_SetEnv(env) < 0) { + return 0; + } + return 1; }