From fe844afa13cae89725fb0f8b5977e9aa50250f06 Mon Sep 17 00:00:00 2001 From: Gabriel Jacobo Date: Mon, 9 Jul 2012 18:08:06 -0300 Subject: [PATCH] Fixes #1422, removes global JNI Env, uses per thread copies, adds thread auto detaching. --- README.android | 14 +++ src/core/android/SDL_android.cpp | 118 +++++++++++++++++--------- src/core/android/SDL_android.h | 6 ++ src/main/android/SDL_android_main.cpp | 6 -- src/thread/pthread/SDL_systhread.c | 6 ++ 5 files changed, 106 insertions(+), 44 deletions(-) diff --git a/README.android b/README.android index c7ffe3505..751200b2e 100644 --- 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 index 0057b2ed7..aa69527c1 100755 --- 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 +#include #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(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 index 5c6a2d8e0..d0f761f4a 100755 --- 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 +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 index 126963cf0..4581e1c3b 100644 --- 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 index 292f7dcab..9ffb611c5 100755 --- 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; }