[Commits] SDL: Fixed bug 1417 - Android_JNI_FileClose local reference bug

[libsdl.org revision control]

details:   http://hg.libsdl.org/SDL/rev/1893d507ba42
changeset: 6284:1893d507ba42
user:      Sam Lantinga <slouken at libsdl.org>
date:      Sun Feb 12 20:57:32 2012 -0500
description:
Fixed bug 1417 - Android_JNI_FileClose local reference bug

A better solution for automatic local reference management.

diffstat:

 src/core/android/SDL_android.cpp |  76 ++++++++++++++++++++++++++++++----------
 1 files changed, 57 insertions(+), 19 deletions(-)

diffs (180 lines):

diff -r 03a7ceb3487b -r 1893d507ba42 src/core/android/SDL_android.cpp
--- a/src/core/android/SDL_android.cpp	Tue Feb 07 19:34:24 2012 -0500
+++ b/src/core/android/SDL_android.cpp	Sun Feb 12 20:57:32 2012 -0500
@@ -20,6 +20,7 @@
 */
 #include "SDL_config.h"
 #include "SDL_stdinc.h"
+#include "SDL_assert.h"
 
 #ifdef __ANDROID__
 
@@ -203,6 +204,41 @@
 /*******************************************************************************
              Functions called by SDL into Java
 *******************************************************************************/
+
+class LocalReferenceHolder
+{
+private:
+    static int s_active;
+
+public:
+    static bool IsActive() {
+        return s_active > 0;
+    }
+
+public:
+    LocalReferenceHolder() : m_env(NULL) { }
+    ~LocalReferenceHolder() {
+        if (m_env) {
+            m_env->PopLocalFrame(NULL);
+            --s_active;
+        }
+    }
+
+    bool init(JNIEnv *env, jint capacity = 16) {
+        if (env->PushLocalFrame(capacity) < 0) {
+            SDL_SetError("Failed to allocate enough JVM local references");
+            return false;
+        }
+        ++s_active;
+        m_env = env;
+        return true;
+    }
+
+protected:
+    JNIEnv *m_env;
+};
+int LocalReferenceHolder::s_active;
+
 extern "C" SDL_bool Android_JNI_CreateContext(int majorVersion, int minorVersion)
 {
     if (mEnv->CallStaticBooleanMethod(mActivityClass, midCreateGLContext, majorVersion, minorVersion)) {
@@ -351,6 +387,8 @@
 // Test for an exception and call SDL_SetError with its detail if one occurs
 static bool Android_JNI_ExceptionOccurred()
 {
+    SDL_assert(LocalReferenceHolder::IsActive());
+
     jthrowable exception = mEnv->ExceptionOccurred();
     if (exception != NULL) {
         jmethodID mid;
@@ -373,16 +411,11 @@
                     exceptionMessage, 0);
             SDL_SetError("%s: %s", exceptionNameUTF8, exceptionMessageUTF8);
             mEnv->ReleaseStringUTFChars(exceptionMessage, exceptionMessageUTF8);
-            mEnv->DeleteLocalRef(exceptionMessage);
         } else {
             SDL_SetError("%s", exceptionNameUTF8);
         }
 
         mEnv->ReleaseStringUTFChars(exceptionName, exceptionNameUTF8);
-        mEnv->DeleteLocalRef(exceptionName);
-        mEnv->DeleteLocalRef(classClass);
-        mEnv->DeleteLocalRef(exceptionClass);
-        mEnv->DeleteLocalRef(exception);
 
         return true;
     }
@@ -392,6 +425,7 @@
 
 static int Android_JNI_FileOpen(SDL_RWops* ctx)
 {
+    LocalReferenceHolder refs;
     int result = 0;
 
     jmethodID mid;
@@ -402,13 +436,8 @@
     jobject readableByteChannel;
     jstring fileNameJString;
 
-    bool allocatedLocalFrame = false;
-
-    if (mEnv->PushLocalFrame(16) < 0) {
-        SDL_SetError("Failed to allocate enough JVM local references");
+    if (!refs.init(mEnv)) {
         goto failure;
-    } else {
-        allocatedLocalFrame = true;
     }
 
     fileNameJString = (jstring)ctx->hidden.androidio.fileName;
@@ -481,16 +510,18 @@
         }
     }
 
-    if (allocatedLocalFrame) {
-        mEnv->PopLocalFrame(NULL);
-    }
-
     return result;
 }
 
 extern "C" int Android_JNI_FileOpen(SDL_RWops* ctx,
         const char* fileName, const char*)
 {
+    LocalReferenceHolder refs;
+
+    if (!refs.init(mEnv)) {
+        return -1;
+    }
+
     if (!ctx) {
         return -1;
     }
@@ -499,7 +530,6 @@
     ctx->hidden.androidio.fileName = fileNameJString;
     ctx->hidden.androidio.fileNameRef = mEnv->NewGlobalRef(fileNameJString);
     ctx->hidden.androidio.inputStreamRef = NULL;
-    mEnv->DeleteLocalRef(fileNameJString);
 
     return Android_JNI_FileOpen(ctx);
 }
@@ -507,9 +537,14 @@
 extern "C" size_t Android_JNI_FileRead(SDL_RWops* ctx, void* buffer,
         size_t size, size_t maxnum)
 {
+    LocalReferenceHolder refs;
     int bytesRemaining = size * maxnum;
     int bytesRead = 0;
 
+    if (!refs.init(mEnv)) {
+        return -1;
+    }
+
     jobject readableByteChannel = (jobject)ctx->hidden.androidio.readableByteChannel;
     jmethodID readMethod = (jmethodID)ctx->hidden.androidio.readMethod;
     jobject byteBuffer = mEnv->NewDirectByteBuffer(buffer, bytesRemaining);
@@ -519,7 +554,6 @@
         int result = mEnv->CallIntMethod(readableByteChannel, readMethod, byteBuffer);
 
         if (Android_JNI_ExceptionOccurred()) {
-            mEnv->DeleteLocalRef(byteBuffer);
             return 0;
         }
 
@@ -532,8 +566,6 @@
         ctx->hidden.androidio.position += result;
     }
 
-    mEnv->DeleteLocalRef(byteBuffer);
-
     return bytesRead / size;
 }
 
@@ -546,8 +578,14 @@
 
 static int Android_JNI_FileClose(SDL_RWops* ctx, bool release)
 {
+    LocalReferenceHolder refs;
     int result = 0;
 
+    if (!refs.init(mEnv)) {
+        SDL_SetError("Failed to allocate enough JVM local references");
+        return -1;
+    }
+
     if (ctx) {
         if (release) {
             mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.fileNameRef);