Committed in r946181. Thanks!
-Hyrum
On Wed, May 19, 2010 at 7:57 AM, Greg Stein <gstein_at_gmail.com> wrote:
> Cool. Thanks!!
>
> On Wed, May 19, 2010 at 00:21, Byeongcheol Lee <lineonking_at_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_wright_at_mail.utexas.edu> wrote:
> >>
> >>
> >> On Tue, May 18, 2010 at 10:14 PM, Byeongcheol Lee <lineonking_at_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
> >>
> >
>
Received on 2010-05-19 15:36:40 CEST