We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

[patch] Fix to Android FS memory leaks

SDL-hg-fix-android-fs-memory-leaks.diff (text/plain), 10.12 KB, created by Tim Angus on 2011-08-26 05:13:17 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tim Angus

Created: 2011-08-26 05:13:17 UTC

Size: 10.12 KB

patch

obsolete

># HG changeset patch
># User Tim Angus <tim@blackcompanystudios.co.uk>
># Date 1314360713 -3600
># Node ID 640c67302f8ef351eacf111807336b0926422e10
># Parent  022880331cac80736d87082752f36ad1e39913c6
>* Fix many memory leaks in Android FS code
>* Set SDL error string with Java exception details when one occurs
>* Fix tabulation of SDLActivity::getContext
>
>diff -r 022880331cac -r 640c67302f8e android-project/src/org/libsdl/app/SDLActivity.java
>--- a/android-project/src/org/libsdl/app/SDLActivity.java	Fri Aug 26 03:38:46 2011 -0400
>+++ b/android-project/src/org/libsdl/app/SDLActivity.java	Fri Aug 26 13:11:53 2011 +0100
>@@ -114,9 +114,9 @@
>         mSingleton.sendCommand(COMMAND_CHANGE_TITLE, title);
>     }
> 
>-	public static Context getContext() {
>-		return mSingleton;
>-	}
>+    public static Context getContext() {
>+        return mSingleton;
>+    }
> 
>     // Audio
>     private static Object buf;
>diff -r 022880331cac -r 640c67302f8e src/core/android/SDL_android.cpp
>--- a/src/core/android/SDL_android.cpp	Fri Aug 26 03:38:46 2011 -0400
>+++ b/src/core/android/SDL_android.cpp	Fri Aug 26 13:11:53 2011 +0100
>@@ -259,35 +259,92 @@
>     }
> }
> 
>+// Test for an exception and call SDL_SetError with its detail if one occurs
>+static bool Android_JNI_ExceptionOccurred()
>+{
>+    jthrowable exception = mEnv->ExceptionOccurred();
>+    if (exception != NULL) {
>+        jmethodID mid;
>+
>+        // Until this happens most JNI operations have undefined behaviour
>+        mEnv->ExceptionClear();
>+
>+        jclass exceptionClass = mEnv->GetObjectClass(exception);
>+        jclass classClass = mEnv->FindClass("java/lang/Class");
>+
>+        mid = mEnv->GetMethodID(classClass, "getName", "()Ljava/lang/String;");
>+        jstring exceptionName = (jstring)mEnv->CallObjectMethod(exceptionClass, mid);
>+        const char* exceptionNameUTF8 = mEnv->GetStringUTFChars(exceptionName, 0);
>+
>+        mid = mEnv->GetMethodID(exceptionClass, "getMessage", "()Ljava/lang/String;");
>+        jstring exceptionMessage = (jstring)mEnv->CallObjectMethod(exceptionClass, mid);
>+
>+        if (exceptionMessage != NULL) {
>+            const char* exceptionMessageUTF8 = mEnv->GetStringUTFChars(
>+                    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;
>+    }
>+
>+    return false;
>+}
>+
> static int Android_JNI_FileOpen(SDL_RWops* ctx)
> {
>-    jstring fileNameJString = (jstring)ctx->hidden.androidio.fileName;
>+    int result = 0;
>+
>+    jmethodID mid;
>+    jobject context;
>+    jobject assetManager;
>+    jobject inputStream;
>+    jclass channels;
>+    jobject readableByteChannel;
>+    jstring fileNameJString;
>+
>+    bool allocatedLocalFrame = false;
>+
>+    if (mEnv->PushLocalFrame(16) < 0) {
>+        SDL_SetError("Failed to allocate enough JVM local references");
>+        goto failure;
>+    } else {
>+        allocatedLocalFrame = true;
>+    }
>+
>+    fileNameJString = (jstring)ctx->hidden.androidio.fileName;
> 
>     // context = SDLActivity.getContext();
>-    jmethodID mid = mEnv->GetStaticMethodID(mActivityClass,
>+    mid = mEnv->GetStaticMethodID(mActivityClass,
>             "getContext","()Landroid/content/Context;");
>-    jobject context = mEnv->CallStaticObjectMethod(mActivityClass, mid);
>+    context = mEnv->CallStaticObjectMethod(mActivityClass, mid);
> 
>     // assetManager = context.getAssets();
>     mid = mEnv->GetMethodID(mEnv->GetObjectClass(context),
>-            "getAssets","()Landroid/content/res/AssetManager;");
>-    jobject assetManager = mEnv->CallObjectMethod(context, mid);
>+            "getAssets", "()Landroid/content/res/AssetManager;");
>+    assetManager = mEnv->CallObjectMethod(context, mid);
> 
>     // inputStream = assetManager.open(<filename>);
>-    mEnv->ExceptionClear();
>     mid = mEnv->GetMethodID(mEnv->GetObjectClass(assetManager),
>             "open", "(Ljava/lang/String;)Ljava/io/InputStream;");
>-    jobject inputStream = mEnv->CallObjectMethod(assetManager, mid, fileNameJString);
>-    if (mEnv->ExceptionOccurred()) {
>-        mEnv->ExceptionDescribe();
>-        mEnv->ExceptionClear();
>-        mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.fileNameRef);
>-        return -1;
>-    } else {
>-        ctx->hidden.androidio.inputStream = inputStream;
>-        ctx->hidden.androidio.inputStreamRef = mEnv->NewGlobalRef(inputStream);
>+    inputStream = mEnv->CallObjectMethod(assetManager, mid, fileNameJString);
>+    if (Android_JNI_ExceptionOccurred()) {
>+        goto failure;
>     }
> 
>+    ctx->hidden.androidio.inputStream = inputStream;
>+    ctx->hidden.androidio.inputStreamRef = mEnv->NewGlobalRef(inputStream);
>+
>     // Store .skip id for seeking purposes
>     mid = mEnv->GetMethodID(mEnv->GetObjectClass(inputStream),
>             "skip", "(J)J");
>@@ -300,38 +357,28 @@
>     // AssetInputStream.available() /will/ always return the total file size
> 
>     // size = inputStream.available();
>-    mEnv->ExceptionClear();
>     mid = mEnv->GetMethodID(mEnv->GetObjectClass(inputStream),
>             "available", "()I");
>     ctx->hidden.androidio.size = mEnv->CallIntMethod(inputStream, mid);
>-    if (mEnv->ExceptionOccurred()) {
>-        mEnv->ExceptionDescribe();
>-        mEnv->ExceptionClear();
>-        mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.fileNameRef);
>-        mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.inputStreamRef);
>-        return -1;
>+    if (Android_JNI_ExceptionOccurred()) {
>+        goto failure;
>     }
> 
>     // readableByteChannel = Channels.newChannel(inputStream);
>-    mEnv->ExceptionClear();
>-    jclass channels = mEnv->FindClass("java/nio/channels/Channels");
>+    channels = mEnv->FindClass("java/nio/channels/Channels");
>     mid = mEnv->GetStaticMethodID(channels,
>             "newChannel",
>             "(Ljava/io/InputStream;)Ljava/nio/channels/ReadableByteChannel;");
>-    jobject readableByteChannel = mEnv->CallStaticObjectMethod(
>+    readableByteChannel = mEnv->CallStaticObjectMethod(
>             channels, mid, inputStream);
>-    if (mEnv->ExceptionOccurred()) {
>-        mEnv->ExceptionDescribe();
>-        mEnv->ExceptionClear();
>-        mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.fileNameRef);
>-        mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.inputStreamRef);
>-        return -1;
>-    } else {
>-        ctx->hidden.androidio.readableByteChannel = readableByteChannel;
>-        ctx->hidden.androidio.readableByteChannelRef =
>-            mEnv->NewGlobalRef(readableByteChannel);
>+    if (Android_JNI_ExceptionOccurred()) {
>+        goto failure;
>     }
> 
>+    ctx->hidden.androidio.readableByteChannel = readableByteChannel;
>+    ctx->hidden.androidio.readableByteChannelRef =
>+        mEnv->NewGlobalRef(readableByteChannel);
>+
>     // Store .read id for reading purposes
>     mid = mEnv->GetMethodID(mEnv->GetObjectClass(readableByteChannel),
>             "read", "(Ljava/nio/ByteBuffer;)I");
>@@ -339,7 +386,22 @@
> 
>     ctx->hidden.androidio.position = 0;
> 
>-    return 0;
>+    if (false) {
>+failure:
>+        result = -1;
>+
>+        mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.fileNameRef);
>+
>+        if(ctx->hidden.androidio.inputStreamRef != NULL) {
>+            mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.inputStreamRef);
>+        }
>+    }
>+
>+    if (allocatedLocalFrame) {
>+        mEnv->PopLocalFrame(NULL);
>+    }
>+
>+    return result;
> }
> 
> extern "C" int Android_JNI_FileOpen(SDL_RWops* ctx,
>@@ -352,6 +414,8 @@
>     jstring fileNameJString = mEnv->NewStringUTF(fileName);
>     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);
> }
>@@ -366,14 +430,12 @@
>     jmethodID readMethod = (jmethodID)ctx->hidden.androidio.readMethod;
>     jobject byteBuffer = mEnv->NewDirectByteBuffer(buffer, bytesRemaining);
> 
>-    mEnv->ExceptionClear();
>     while (bytesRemaining > 0) {
>         // result = readableByteChannel.read(...);
>         int result = mEnv->CallIntMethod(readableByteChannel, readMethod, byteBuffer);
> 
>-        if (mEnv->ExceptionOccurred()) {
>-            mEnv->ExceptionDescribe();
>-            mEnv->ExceptionClear();
>+        if (Android_JNI_ExceptionOccurred()) {
>+            mEnv->DeleteLocalRef(byteBuffer);
>             return 0;
>         }
> 
>@@ -386,6 +448,8 @@
>         ctx->hidden.androidio.position += result;
>     }
> 
>+    mEnv->DeleteLocalRef(byteBuffer);
>+
>     return bytesRead / size;
> }
> 
>@@ -408,16 +472,13 @@
>         jobject inputStream = (jobject)ctx->hidden.androidio.inputStream;
> 
>         // inputStream.close();
>-        mEnv->ExceptionClear();
>         jmethodID mid = mEnv->GetMethodID(mEnv->GetObjectClass(inputStream),
>                 "close", "()V");
>         mEnv->CallVoidMethod(inputStream, mid);
>         mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.inputStreamRef);
>         mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.readableByteChannelRef);
>-        if (mEnv->ExceptionOccurred()) {
>+        if (Android_JNI_ExceptionOccurred()) {
>             result = -1;
>-            mEnv->ExceptionDescribe();
>-            mEnv->ExceptionClear();
>         }
> 
>         if (release) {
>@@ -460,14 +521,10 @@
> 
>     if (movement > 0) {
>         // The easy case where we're seeking forwards
>-        mEnv->ExceptionClear();
>         while (movement > 0) {
>             // inputStream.skip(...);
>             movement -= mEnv->CallLongMethod(inputStream, skipMethod, movement);
>-            if (mEnv->ExceptionOccurred()) {
>-                mEnv->ExceptionDescribe();
>-                mEnv->ExceptionClear();
>-                SDL_SetError("Exception while seeking");
>+            if (Android_JNI_ExceptionOccurred()) {
>                 return -1;
>             }
>         }

Actions: View | Diff

Attachments on bug 1261: 662 | 682 | 684

We are currently migrating Bugzilla to GitHub issues.Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.