On Wed, May 19, 2010 at 6:26 PM, Byeongcheol Lee <lineonking_at_gmail.com>wrote:
> 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<http://userweb.cs.utexas.edu/%7Ebclee/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.
>
Wow. Thanks for finding this error. Fix committed in r946518.
Cheers,
-Hyrum
Received on 2010-05-20 06:08:16 CEST