Fixes #1422, removes global JNI Env, uses per thread copies, adds thread auto detaching.
authorGabriel Jacobo <gabomdq@gmail.com>
Mon, 09 Jul 2012 18:08:06 -0300
changeset 6354 17840f487124
parent 6352 a9bcd26e7105
child 6355 3ef4d0e923cb
Fixes #1422, removes global JNI Env, uses per thread copies, adds thread auto detaching.
README.android
src/core/android/SDL_android.cpp
src/core/android/SDL_android.h
src/main/android/SDL_android_main.cpp
src/thread/pthread/SDL_systhread.c
--- a/README.android	Thu Jul 05 12:16:44 2012 -0400
+++ b/README.android	Mon Jul 09 18:08:06 2012 -0300
@@ -92,6 +92,20 @@
 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
 ================================================================================
 
--- a/src/core/android/SDL_android.cpp	Thu Jul 05 12:16:44 2012 -0400
+++ b/src/core/android/SDL_android.cpp	Mon Jul 09 18:08:06 2012 -0300
@@ -33,6 +33,7 @@
 #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 @@
 /*******************************************************************************
                                Globals
 *******************************************************************************/
-static JNIEnv* mEnv = NULL;
-static JNIEnv* mAudioEnv = NULL;
+static pthread_key_t mThreadKey;
 static JavaVM* mJavaVM;
 
 // Main activity
@@ -87,17 +87,28 @@
         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 @@
                                     JNIEnv* env, jclass cls)
 {
     /* This is the audio thread, with a different environment */
-    mAudioEnv = env;
+    Android_JNI_SetupThread();
 
     Android_RunAudioThread();
 }
@@ -248,6 +259,7 @@
 
 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" 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 @@
     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 @@
     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 @@
         audioBufferFrames /= 2;
     }
  
-    if (isAttached) {
-        mJavaVM->DetachCurrentThread();
-    }
-
     return audioBufferFrames;
 }
 
@@ -353,6 +403,8 @@
 
 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_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 @@
         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 @@
     jobject readableByteChannel;
     jstring fileNameJString;
 
+    JNIEnv *mEnv = Android_JNI_GetEnv();
     if (!refs.init(mEnv)) {
         goto failure;
     }
@@ -529,6 +568,7 @@
         const char* fileName, const char*)
 {
     LocalReferenceHolder refs;
+    JNIEnv *mEnv = Android_JNI_GetEnv();
 
     if (!refs.init(mEnv)) {
         return -1;
@@ -554,6 +594,7 @@
     int bytesRemaining = size * maxnum;
     int bytesRead = 0;
 
+    JNIEnv *mEnv = Android_JNI_GetEnv();
     if (!refs.init(mEnv)) {
         return -1;
     }
@@ -593,6 +634,7 @@
 {
     LocalReferenceHolder refs;
     int result = 0;
+    JNIEnv *mEnv = Android_JNI_GetEnv();
 
     if (!refs.init(mEnv)) {
         SDL_SetError("Failed to allocate enough JVM local references");
--- a/src/core/android/SDL_android.h	Thu Jul 05 12:16:44 2012 -0400
+++ b/src/core/android/SDL_android.h	Mon Jul 09 18:08:06 2012 -0300
@@ -47,6 +47,12 @@
 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* */
--- a/src/main/android/SDL_android_main.cpp	Thu Jul 05 12:16:44 2012 -0400
+++ b/src/main/android/SDL_android_main.cpp	Mon Jul 09 18:08:06 2012 -0300
@@ -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)
 {
--- a/src/thread/pthread/SDL_systhread.c	Thu Jul 05 12:16:44 2012 -0400
+++ b/src/thread/pthread/SDL_systhread.c	Mon Jul 09 18:08:06 2012 -0300
@@ -40,6 +40,9 @@
 #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 void *
 RunThread(void *data)
 {
+#ifdef __ANDROID__
+    Android_JNI_SetupThread();
+#endif
     SDL_RunThread(data);
     return NULL;
 }