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

[Byeongcheol Lee <li...@gmail.com>]

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