[PATCH] Fix for a local frame leak in EnumMapper.cpp

[Byeongcheol Lee]

Dear Subversion Developers:

I attach a patch for a local frame leak in EnumMapper.cpp, and the log
message follows here:

[[[
* subversion/bindings/javahl/native/CreateJ.cpp
 (getOrdinal): add a call to "PushLocalFrame" before the method
returns successfully without a Java exception.
]]]

To reproduce this bug, run your javaHL regression test under our
dynamic bug detector, JInn [http://userweb.cs.utexas.edu/~bclee/jinn]:

$env JAVA_TOOL_OPTIONS=-agentlib:jinn make check-javahl
...
Picked up JAVA_TOOL_OPTIONS: -agentlib:jinn
.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.
E.E.E.E.E.E.E.E.E.E.E
Time: 6.371
There were 51 errors:
1) 
testCreate(org.apache.subversion.javahl.SVNAdminTests)xtc.lang.blink.agent.JNIAssertionFailure:
Error: leak where local frame is not empty.
       at 
xtc.lang.blink.agent.JNIAssertionFailure.assertFail(JNIAssertionFailure.java:16)
       at org.apache.subversion.javahl.SVNClient.doImport(Native Method)
       at org.apache.subversion.javahl.SVNTests.setUp(SVNTests.java:239)
       at org.apache.subversion.javahl.RunTests.main(RunTests.java:116)
2) 
testSetRevProp(org.apache.subversion.javahl.SVNAdminTests)xtc.lang.blink.agent.JNIAssertionFailure:
Error: leak where local frame is not empty.
       at 
xtc.lang.blink.agent.JNIAssertionFailure.assertFail(JNIAssertionFailure.java:16)
       at org.apache.subversion.javahl.SVNClient.doImport(Native Method)
       at org.apache.subversion.javahl.SVNTests.setUp(SVNTests.java:239)
       at org.apache.subversion.javahl.RunTests.main(RunTests.java:116)
...

The local frame leak here happens when the number of calls to
"PushLocalFrame" is strictly more than "PopLocalFrame" when a native
method returns to JVM. This means that there was a call to
"PushLocalFrame" that does not have a corresponding call to
"PopLocalFrame" within the native method "SVNClicent.doImport" This
program run violates JNI programming rule:

 "Failing to place PopLocalFrame calls properly would lead to
undefined behavior, such as virtual machine crashes. "
 [http://java.sun.com/docs/books/jni/html/refs.html#27623]

I attached gdb the JVM and examined every call to "PushLocalFrame",
and  I ended up with the source line 229 in EnumMapper.cpp:

In subversion/bindings/javahl/native/EnumMapper.cpp
  224  int EnumMapper::getOrdinal(const char *clazzName, jobject jenum)
  225  {
  226    JNIEnv *env = JNIUtil::getEnv();
  227
  228    // Create a local frame for our references
  229    env->PushLocalFrame(LOCAL_FRAME_SIZE);
  230    if (JNIUtil::isJavaExceptionThrown())
  231      return -1;
  232
  233    jclass clazz = env->FindClass(clazzName);
  234    if (JNIUtil::isJavaExceptionThrown())
  235      POP_AND_RETURN(-1);
  236
  237    jmethodID mid = env->GetMethodID(clazz, "ordinal", "()I");
  238    if (JNIUtil::isJavaExceptionThrown())
  239      POP_AND_RETURN(-1);
  240
  241    jint jorder = env->CallIntMethod(jenum, mid);
  242    if (JNIUtil::isJavaExceptionThrown())
  243      POP_AND_RETURN(-1);
  244
  245    return (int) jorder;
  246  }

When the C++ method "getOrdinal" does not receive any Java exception
from JNI calls at Line 233, 237, and 241, it does not
pop up the current local frame allocated at line 229.

Regards,
Byeong[PATCH] Fix for a local frame leak in EnumMapper.cpp

Index: subversion/bindings/javahl/native/EnumMapper.cpp
===================================================================
--- subversion/bindings/javahl/native/EnumMapper.cpp	(revision 946319)
+++ subversion/bindings/javahl/native/EnumMapper.cpp	(working copy)
@@ -242,5 +242,6 @@
   if (JNIUtil::isJavaExceptionThrown())
     POP_AND_RETURN(-1);
 
+  env->PopLocalFrame(NULL);
   return (int) jorder;
 }