Can you pinpoint where the local reference(s) are being created/leaked? I can't see it. This patch may fix the problem, but I think it's important to understand the cause in case leaks also exist elsewhere. Well done on narrowing it down so far though.

Hmm...

diff -r 03a7ceb3487b 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  Sat Feb 11 22:12:23 2012 +0000
@@ -559,6 +559,7 @@
         jmethodID mid = mEnv->GetMethodID(mEnv->GetObjectClass(inputStream),
                 "close", "()V");
         mEnv->CallVoidMethod(inputStream, mid);
+        mEnv->DeleteLocalRef(jmethodID);
         mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.inputStreamRef);
         mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.readableByteChannelRef);
         if (Android_JNI_ExceptionOccurred()) {

I'm not in a position to test this until next week, it would be interesting to see if it has the same effect as the pop/push patch.

Actually, if that is the cause, the results of GetMethodID in the exception handling function would need to be deleted too. If this is the case it would mean that the read function is leaking references also.

I think that, while it is an interesting exercise, it's not really vital knowing which local reference we are forgetting to release. My vote is to enclose every one of these functions in a Push/Pop local frame, as this type of bugs are hard to detect and I think there's no unit tests for RWops in SDL Android...also I don't know of any downside of doing it like this...
It seems to me it's also a good defensive mechanism for potential future modifications by other programmers that haven't gone through this issues already.

BTW, at some point while debugging this I did a test by releasing mid, but I don't think that was the cause.

The disadvantages of using push/pop are that it's an inefficient use of local refs, which could potentially lead to exhausting them prematurely. If you manage refs manually, you only ever take exactly what you need... except when you're leaking them of course :). The other thing is that for functions that have mutiple return statements, each must pop the frame. In other words, the control flow is potentially more complicated.

Having said that, you're probably right, but push/pop should be added to all functions (including ones not related to file IO) and any DeleteLocalRef calls removed. By the way, in your existing patch the allocatedLocalFrame variable is unnecessary; it'll always be true when it is used in the if statement.

If what you say about GetMethodID not being the leak is true, I'm pretty mystified where the leak is coming from.

Hey Tim, this is the true fix:

jobject inputStream = (jobject)ctx->hidden.androidio.inputStream;
 // inputStream.close();
 jclass inputStreamClass = mEnv->GetObjectClass(inputStream);
 jmethodID mid = mEnv->GetMethodID(inputStreamClass,
 "close", "()V");
 mEnv->CallVoidMethod(inputStream, mid);
 mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.inputStreamRef);
 mEnv->DeleteGlobalRef((jobject)ctx->hidden.androidio.readableByteChannelRef);
 mEnv->DeleteLocalRef((jobject)inputStreamClass);


It turns out mEnv->GetObjectClass(inputStream) creates a local ref.

Created attachment 820 [details]
Patch to automatically manage local references

Here's a completely untested patch to help automate local reference management.

(In reply to comment #7) 
> It turns out mEnv->GetObjectClass(inputStream) creates a local ref.

Bah, how did I miss that? Too much beer.

Sam your patch looks ideal. It's easy to forget this is a C++ file.

The patch compiles fine if you #include SDL_assert.h in core/android/SDL_android.h (or somewhere else...the config file perhaps?) and it solves the issue perfectly.
I must say it's a very elegant solution and perhaps we should add it to the other JNI functions, like the one that handles Exceptions and the audio ones as well.
+1 from me

Okay, thanks!
http://hg.libsdl.org/SDL/rev/1893d507ba42