On Wed, May 19, 2010 at 6:26 PM, Byeongcheol Lee <lineonk...@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
