Fixes #1422, removes global JNI Env, uses per thread copies, adds thread auto detaching.
--- 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;
}