Committed in r946181. Thanks! -Hyrum
On Wed, May 19, 2010 at 7:57 AM, Greg Stein <gst...@gmail.com> wrote: > 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<http://userweb.cs.utexas.edu/%7Ebclee/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 > >> > > >
