Cool. Thanks!!
On Wed, May 19, 2010 at 00:21, Byeongcheol Lee <lineonk...@gmail.com> wrote: > [[[ > * subversion/bindings/javahl/native/CreateJ.cpp > (Set): Use CallBooleanMethod for a method returning a boolean value. > ]]] > > On Tue, May 18, 2010 at 10:25 PM, Hyrum K. Wright > <hyrum_wri...@mail.utexas.edu> wrote: >> >> >> On Tue, May 18, 2010 at 10:14 PM, Byeongcheol Lee <lineonk...@gmail.com> >> wrote: >>> >>> Dear Subversion Developers: >>> >>> I attach a patch for a JNI bug. To reproduce the bug, run your JavaHL >>> regression test under our dynamic JNI bug detector, Jinn >>> [http://userweb.cs.utexas.edu/~bclee/jinn]. >>> >>> $env JAVA_TOOL_OPTIONS=-agentlib:jinn make check-javahl >>> ... >>> .Exception in thread "main" xtc.lang.blink.agent.JNIAssertionFailure: >>> The return type mismatch in CallObjectMethodV: method 0x9626fe4 has >>> return type: (Ljava/lang/Object;)Z >>> 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 junit.framework.TestCase.runBare(TestCase.java:128) >>> at junit.framework.TestResult$1.protect(TestResult.java:106) >>> at junit.framework.TestResult.runProtected(TestResult.java:124) >>> at junit.framework.TestResult.run(TestResult.java:109) >>> at junit.framework.TestCase.run(TestCase.java:120) >>> at junit.framework.TestSuite.runTest(TestSuite.java:230) >>> at junit.framework.TestSuite.run(TestSuite.java:225) >>> at junit.framework.TestSuite.runTest(TestSuite.java:230) >>> at junit.framework.TestSuite.run(TestSuite.java:225) >>> at junit.textui.TestRunner.doRun(TestRunner.java:121) >>> at junit.textui.TestRunner.doRun(TestRunner.java:114) >>> at junit.textui.TestRunner.run(TestRunner.java:77) >>> at org.apache.subversion.javahl.RunTests.main(RunTests.java:116) >>> ... >>> >>> To understand what caused the JNI error, I attached a gdb to the JVM. >>> Relevant caling context and source lines are here. >>> >>> #5 0x8f86cdf5 in CreateJ::Set (objects=...) >>> at subversion/bindings/javahl/native/CreateJ.cpp:912 >>> #6 0x8f86bf4f in CommitMessage::getCommitMessage (this=0x81bf6d0, >>> commit_items=0x817ea60) >>> at subversion/bindings/javahl/native/CommitMessage.cpp:210 >>> ... >>> #11 0x8f88bff0 in Java_org_apache_subversion_javahl_SVNClient_doImport ( >>> env=0x805a518, jthis=0xb7281ce4, jpath=0xb7281ce0, jurl=0xb7281cdc, >>> jmessage=0x0, jdepth=0xb7281cd4, jnoIgnore=0 '\000', >>> jignoreUnknownNodeTypes=0 '\000', jrevpropTable=0x0) >>> at >>> subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNClient.cpp:792 >>> >>> In subversion/bindings/javahl/native/CreateJ.cpp >>> ... >>> 883 jclass clazz = env->FindClass("java/util/HashSet"); >>> ... >>> 895 static jmethodID add_mid = 0; >>> ... >>> 898 add_mid = env->GetMethodID(clazz, "add", >>> "(Ljava/lang/Object;)Z"); >>> ... >>> 912 env->CallObjectMethod(set, add_mid, jthing); >>> ... >>> >>> >>> There is a JNI type mismatch between Line 898 and 912. >>> The "add_mid" at Line 898 points a Java method returning a boolean >>> value, but the >>> JNI function at Line 912 assumes that "add_mid" is returning a Java >>> reference. >>> This violates a JNI programming rule: >>> >>> "You should replace type in Call<type>Method with the Java type of the >>> method you are calling" >>> >>> [http://java.sun.com/javase/6/docs/technotes/guides/jni/spec/functions.html#wp4256] >>> >>> The fix is trivial and obvious, and line 912 must change its JNI >>> function from "CallObjectMethod" >>> to "CallBooleanMethod." >> >> The patch looks sound. Would you write a log message, per the patch >> submission guidelines[1]? You can find our log message format at [2]. >> >> Thanks! >> >> -Hyrum >> >> [1] http://subversion.apache.org/docs/community-guide/general.html#patches >> [2] >> http://subversion.apache.org/docs/community-guide/conventions.html#log-messages >> >
