Fixes #1422, removes global JNI Env, uses per thread copies, adds thr…

…ead auto detaching.
libsdl-org · Jul 9, 2012 · fe844af · fe844af
1 parent 661cd17
commit fe844af
Show file tree

Hide file tree

Showing 5 changed files with 106 additions and 44 deletions.
diff --git a/README.android b/README.android
@@ -91,6 +91,20 @@ a specific message, (which is not yet implemented!) and restore your textures
 manually or quit the app (which is actually the kind of behaviour you'll see
 under iOS, if the OS can not restore your GL context it will just kill your app)
 
+================================================================================
+ Threads and the JAVA VM
+================================================================================
+
+For a quick tour on how Linux native threads interoperate with the JAVA VM, take
+a look here: http://developer.android.com/guide/practices/jni.html
+If you want to use threads in your SDL app, it's strongly recommended that you
+do so by creating them using SDL functions. This way, the required attach/detach
+handling is managed by SDL automagically. If you have threads created by other
+means and they make calls to SDL functions, make sure that you call
+Android_JNI_SetupThread before doing anything else otherwise SDL will attach
+your thread automatically anyway (when you make an SDL call), but it'll never
+detach it.
+
 ================================================================================
  Additional documentation
 ================================================================================

diff --git a/src/core/android/SDL_android.cpp b/src/core/android/SDL_android.cpp
@@ -33,6 +33,7 @@ extern "C" {
 #include "../../video/android/SDL_androidvideo.h"
 
 #include <android/log.h>
+#include <pthread.h>
 #define LOG_TAG "SDL_android"
 //#define LOGI(...)  __android_log_print(ANDROID_LOG_INFO,LOG_TAG,__VA_ARGS__)
 //#define LOGE(...)  __android_log_print(ANDROID_LOG_ERROR,LOG_TAG,__VA_ARGS__)
@@ -54,8 +55,7 @@ extern void Android_RunAudioThread();
 /*******************************************************************************
                                Globals
 *******************************************************************************/
-static JNIEnv* mEnv = NULL;
-static JNIEnv* mAudioEnv = NULL;
+static pthread_key_t mThreadKey;
 static JavaVM* mJavaVM;
 
 // Main activity
@@ -87,17 +87,28 @@ extern "C" jint JNI_OnLoad(JavaVM* vm, void* reserved)
         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
+     */
+    if (pthread_key_create(&mThreadKey, Android_JNI_ThreadDestroyed)) {
+        __android_log_print(ANDROID_LOG_ERROR, "SDL", "Error initializing pthread key");
+    }
+    else {
+        Android_JNI_SetupThread();
+    }
 
     return JNI_VERSION_1_4;
 }
 
 // Called before SDL_main() to initialize JNI bindings
-extern "C" void SDL_Android_Init(JNIEnv* env, jclass cls)
+extern "C" void SDL_Android_Init(JNIEnv* mEnv, jclass cls)
 {
     __android_log_print(ANDROID_LOG_INFO, "SDL", "SDL_Android_Init()");
 
-    mEnv = env;
-    mActivityClass = (jclass)env->NewGlobalRef(cls);
+    Android_JNI_SetupThread();
+
+    mActivityClass = (jclass)mEnv->NewGlobalRef(cls);
 
     midCreateGLContext = mEnv->GetStaticMethodID(mActivityClass,
                                 "createGLContext","(II)Z");
@@ -202,7 +213,7 @@ extern "C" void Java_org_libsdl_app_SDLActivity_nativeRunAudioThread(
                                     JNIEnv* env, jclass cls)
 {
     /* This is the audio thread, with a different environment */
-    mAudioEnv = env;
+    Android_JNI_SetupThread();
 
     Android_RunAudioThread();
 }
@@ -248,6 +259,7 @@ int LocalReferenceHolder::s_active;
 
 extern "C" SDL_bool Android_JNI_CreateContext(int majorVersion, int minorVersion)
 {
+    JNIEnv *mEnv = Android_JNI_GetEnv();
     if (mEnv->CallStaticBooleanMethod(mActivityClass, midCreateGLContext, majorVersion, minorVersion)) {
         return SDL_TRUE;
     } else {
@@ -257,13 +269,14 @@ extern "C" SDL_bool Android_JNI_CreateContext(int majorVersion, int minorVersion
 
 extern "C" void Android_JNI_SwapWindow()
 {
+    JNIEnv *mEnv = Android_JNI_GetEnv();
     mEnv->CallStaticVoidMethod(mActivityClass, midFlipBuffers); 
 }
 
 extern "C" void Android_JNI_SetActivityTitle(const char *title)
 {
     jmethodID mid;
-
+    JNIEnv *mEnv = Android_JNI_GetEnv();
     mid = mEnv->GetStaticMethodID(mActivityClass,"setActivityTitle","(Ljava/lang/String;)V");
     if (mid) {
         jstring jtitle = reinterpret_cast<jstring>(mEnv->NewStringUTF(title));
@@ -288,6 +301,53 @@ extern "C" SDL_bool Android_JNI_GetAccelerometerValues(float values[3])
     return retval;
 }
 
+static void Android_JNI_ThreadDestroyed(void* value) {
+    /* The thread is being destroyed, detach it from the Java VM and set the mThreadKey value to NULL as required */
+    JNIEnv *env = (JNIEnv*) value;
+    if (env != NULL) {
+        mJavaVM->DetachCurrentThread();
+        pthread_setspecific(mThreadKey, NULL);
+    }
+}
+
+JNIEnv* Android_JNI_GetEnv(void) {
+    /* From http://developer.android.com/guide/practices/jni.html
+     * All threads are Linux threads, scheduled by the kernel.
+     * They're usually started from managed code (using Thread.start), but they can also be created elsewhere and then
+     * attached to the JavaVM. For example, a thread started with pthread_create can be attached with the
+     * JNI AttachCurrentThread or AttachCurrentThreadAsDaemon functions. Until a thread is attached, it has no JNIEnv,
+     * and cannot make JNI calls.
+     * Attaching a natively-created thread causes a java.lang.Thread object to be constructed and added to the "main"
+     * ThreadGroup, making it visible to the debugger. Calling AttachCurrentThread on an already-attached thread
+     * is a no-op.
+     * 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(&env, NULL);
+    if(status < 0) {
+        LOGE("failed to attach current thread");
+        return 0;
+    }
+
+    return env;
+}
+
+int Android_JNI_SetupThread(void) {
+    /* 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
+     * called before the thread exits, and call DetachCurrentThread from there. (Use that key with pthread_setspecific
+     * to store the JNIEnv in thread-local-storage; that way it'll be passed into your destructor as the argument.)
+     * Note: The destructor is not called unless the stored value is != NULL
+     * 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)
+     */
+    JNIEnv *env = Android_JNI_GetEnv();
+    pthread_setspecific(mThreadKey, (void*) env);
+    return 1;
+}
+
 //
 // Audio support
 //
@@ -301,18 +361,12 @@ extern "C" int Android_JNI_OpenAudioDevice(int sampleRate, int is16Bit, int chan
     int audioBufferFrames;
 
     int status;
-    JNIEnv *env;
-    static bool isAttached = false;    
-    status = mJavaVM->GetEnv((void **) &env, JNI_VERSION_1_4);
-    if(status < 0) {
-        LOGE("callback_handler: failed to get JNI environment, assuming native thread");
-        status = mJavaVM->AttachCurrentThread(&env, NULL);
-        if(status < 0) {
-            LOGE("callback_handler: failed to attach current thread");
-            return 0;
-        }
-        isAttached = true;
+    JNIEnv *env = Android_JNI_GetEnv();
+
+    if (!env) {
+        LOGE("callback_handler: failed to attach current thread");
     }
+    Android_JNI_SetupThread();
 
 
     __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "SDL audio: opening device");
@@ -339,10 +393,6 @@ extern "C" int Android_JNI_OpenAudioDevice(int sampleRate, int is16Bit, int chan
         audioBufferFrames /= 2;
     }
 
-    if (isAttached) {
-        mJavaVM->DetachCurrentThread();
-    }
-
     return audioBufferFrames;
 }
 
@@ -353,6 +403,8 @@ extern "C" void * Android_JNI_GetAudioBuffer()
 
 extern "C" void Android_JNI_WriteAudioBuffer()
 {
+    JNIEnv *mAudioEnv = Android_JNI_GetEnv();
+
     if (audioBuffer16Bit) {
         mAudioEnv->ReleaseShortArrayElements((jshortArray)audioBuffer, (jshort *)audioBufferPinned, JNI_COMMIT);
         mAudioEnv->CallStaticVoidMethod(mActivityClass, midAudioWriteShortBuffer, (jshortArray)audioBuffer);
@@ -367,18 +419,7 @@ extern "C" void Android_JNI_WriteAudioBuffer()
 extern "C" void Android_JNI_CloseAudioDevice()
 {
     int status;
-    JNIEnv *env;
-    static bool isAttached = false;    
-    status = mJavaVM->GetEnv((void **) &env, JNI_VERSION_1_4);
-    if(status < 0) {
-        LOGE("callback_handler: failed to get JNI environment, assuming native thread");
-        status = mJavaVM->AttachCurrentThread(&env, NULL);
-        if(status < 0) {
-            LOGE("callback_handler: failed to attach current thread");
-            return;
-        }
-        isAttached = true;
-    }
+    JNIEnv *env = Android_JNI_GetEnv();
 
     env->CallStaticVoidMethod(mActivityClass, midAudioQuit); 
 
@@ -387,16 +428,13 @@ extern "C" void Android_JNI_CloseAudioDevice()
         audioBuffer = NULL;
         audioBufferPinned = NULL;
     }
-
-    if (isAttached) {
-        mJavaVM->DetachCurrentThread();
-    }
 }
 
 // Test for an exception and call SDL_SetError with its detail if one occurs
 static bool Android_JNI_ExceptionOccurred()
 {
     SDL_assert(LocalReferenceHolder::IsActive());
+    JNIEnv *mEnv = Android_JNI_GetEnv();
 
     jthrowable exception = mEnv->ExceptionOccurred();
     if (exception != NULL) {
@@ -445,6 +483,7 @@ static int Android_JNI_FileOpen(SDL_RWops* ctx)
     jobject readableByteChannel;
     jstring fileNameJString;
 
+    JNIEnv *mEnv = Android_JNI_GetEnv();
     if (!refs.init(mEnv)) {
         goto failure;
     }
@@ -529,6 +568,7 @@ extern "C" int Android_JNI_FileOpen(SDL_RWops* ctx,
         const char* fileName, const char*)
 {
     LocalReferenceHolder refs;
+    JNIEnv *mEnv = Android_JNI_GetEnv();
 
     if (!refs.init(mEnv)) {
         return -1;
@@ -554,6 +594,7 @@ extern "C" size_t Android_JNI_FileRead(SDL_RWops* ctx, void* buffer,
     int bytesRemaining = size * maxnum;
     int bytesRead = 0;
 
+    JNIEnv *mEnv = Android_JNI_GetEnv();
     if (!refs.init(mEnv)) {
         return -1;
     }
@@ -593,6 +634,7 @@ static int Android_JNI_FileClose(SDL_RWops* ctx, bool release)
 {
     LocalReferenceHolder refs;
     int result = 0;
+    JNIEnv *mEnv = Android_JNI_GetEnv();
 
     if (!refs.init(mEnv)) {
         SDL_SetError("Failed to allocate enough JVM local references");

diff --git a/src/core/android/SDL_android.h b/src/core/android/SDL_android.h
@@ -47,6 +47,12 @@ size_t Android_JNI_FileRead(SDL_RWops* ctx, void* buffer, size_t size, size_t ma
 size_t Android_JNI_FileWrite(SDL_RWops* ctx, const void* buffer, size_t size, size_t num);
 int Android_JNI_FileClose(SDL_RWops* ctx);
 
+// Threads
+#include <jni.h>
+static void Android_JNI_ThreadDestroyed(void*);
+JNIEnv *Android_JNI_GetEnv(void);
+int Android_JNI_SetupThread(void);
+
 /* Ends C function definitions when using C++ */
 #ifdef __cplusplus
 /* *INDENT-OFF* */

diff --git a/src/main/android/SDL_android_main.cpp b/src/main/android/SDL_android_main.cpp
@@ -14,12 +14,6 @@
 // Called before SDL_main() to initialize JNI bindings in SDL library
 extern "C" void SDL_Android_Init(JNIEnv* env, jclass cls);
 
-// Library init
-extern "C" jint JNI_OnLoad(JavaVM* vm, void* reserved)
-{
-    return JNI_VERSION_1_4;
-}
-
 // Start up the SDL app
 extern "C" void Java_org_libsdl_app_SDLActivity_nativeInit(JNIEnv* env, jclass cls, jobject obj)
 {

diff --git a/src/thread/pthread/SDL_systhread.c b/src/thread/pthread/SDL_systhread.c
@@ -40,6 +40,9 @@ extern int pthread_setname_np (pthread_t __target_thread, __const char *__name)
 #include "SDL_thread.h"
 #include "../SDL_thread_c.h"
 #include "../SDL_systhread.h"
+#ifdef __ANDROID__
+#include "../../core/android/SDL_android.h"
+#endif
 
 /* List of signals to mask in the subthreads */
 static const int sig_list[] = {
@@ -51,6 +54,9 @@ static const int sig_list[] = {
 static void *
 RunThread(void *data)
 {
+#ifdef __ANDROID__
+    Android_JNI_SetupThread();
+#endif
     SDL_RunThread(data);
     return NULL;
 }