You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Byeongcheol Lee <li...@gmail.com> on 2010/05/19 03:14:17 UTC

[PATCH] Fix for a JNI bug in CreateJ.cpp (JavaHL)

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."

Regards,
Byeong

Re: [PATCH] Fix for a JNI bug in CreateJ.cpp (JavaHL)

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Committed in r946181.  Thanks!

-Hyrum

On Wed, May 19, 2010 at 7:57 AM, Greg Stein <gs...@gmail.com> wrote:

> Cool. Thanks!!
>
> On Wed, May 19, 2010 at 00:21, Byeongcheol Lee <li...@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
> > <hy...@mail.utexas.edu> wrote:
> >>
> >>
> >> On Tue, May 18, 2010 at 10:14 PM, Byeongcheol Lee <lineonking@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
> >>
> >
>

Re: [PATCH] Fix for a JNI bug in CreateJ.cpp (JavaHL)

Posted by Greg Stein <gs...@gmail.com>.
Cool. Thanks!!

On Wed, May 19, 2010 at 00:21, Byeongcheol Lee <li...@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
> <hy...@mail.utexas.edu> wrote:
>>
>>
>> On Tue, May 18, 2010 at 10:14 PM, Byeongcheol Lee <li...@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
>>
>

Re: [PATCH] Fix for a JNI bug in CreateJ.cpp (JavaHL)

Posted by Byeongcheol Lee <li...@gmail.com>.
[[[
* 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
<hy...@mail.utexas.edu> wrote:
>
>
> On Tue, May 18, 2010 at 10:14 PM, Byeongcheol Lee <li...@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
>

Re: [PATCH] Fix for a JNI bug in CreateJ.cpp (JavaHL)

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Tue, May 18, 2010 at 10:14 PM, Byeongcheol Lee <li...@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