You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by am...@apache.org on 2020/10/15 10:14:30 UTC
svn commit: r1882520 -
/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
Author: amiloslavskiy
Date: Thu Oct 15 10:14:30 2020
New Revision: 1882520
URL: http://svn.apache.org/viewvc?rev=1882520&view=rev
Log:
JavaHL: Fix 'JNI call made with exception pending' error
It is incorrect to call Java method with 'CallObjectMethod' when there
is an unhandled Java exception already pending. The fix is to
temporarily move exception out of the way.
This error is easily seen when running JavaHL tests.
[in subversion/bindings/javahl]
* native/JNIUtil.cpp
Use 'StashException' helper class to temporarily move exception out
of the way in two functions which are designed to run when there is
an exception pending.
Modified:
subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
Modified: subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
URL: http://svn.apache.org/viewvc/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp?rev=1882520&r1=1882519&r2=1882520&view=diff
==============================================================================
--- subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp (original)
+++ subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp Thu Oct 15 10:14:30 2020
@@ -551,6 +551,11 @@ std::string JNIUtil::makeSVNErrorMessage
jstring *jerror_message,
jobject *jmessage_stack)
{
+ // This function may be called with a pending Java exception.
+ // It is incorrect to call Java methods (see code below) with a pending
+ // exception. Stash it away until this function exits.
+ StashException stash(getEnv());
+
if (jerror_message)
*jerror_message = NULL;
if (jmessage_stack)
@@ -761,7 +766,13 @@ namespace {
const char* known_exception_to_cstring(apr_pool_t* pool)
{
JNIEnv *env = JNIUtil::getEnv();
+
+ // This function may be called with a pending Java exception.
+ // It is incorrect to call Java methods (see code below) with a pending
+ // exception. Stash it away until this function exits.
jthrowable t = env->ExceptionOccurred();
+ StashException stashed(env);
+
jclass cls = env->GetObjectClass(t);
jstring jclass_name;
Re: svn commit: r1882520 - /subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
Posted by Johan Corveleyn <jc...@gmail.com>.
On Wed, Jan 27, 2021 at 10:16 PM Alexandr Miloslavskiy
<al...@syntevo.com> wrote:
>
> Thanks for reviewing! You're quite the savior of this branch :)
Heh, you're welcome :). And thanks for bringing it here in the first place.
> On 27.01.2021 11:02, Johan Corveleyn wrote:
> > was wondering what you meant by "This error is easily seen
> > when running JavaHL tests"
>
> If you checkout r1882518, tests die with 'Bad global or local ref passed
> to JNI' before this error can be seen. However, if you also cherry-pick
> r1882522 on top to fix this problem, that's where you'll see 'JNI call
> made with exception pending' that I fixed in r1882520.
Ah, okay. I'll give that another try.
> Sorry for confusion. I edited/reordered patches a few times before
> committing, and didn't notice that the commit message is not too good
> with this ordering.
>
> Which brings me to next question. In other replies, you suggested that I
> make some edits in code and also amend commit messages. How do I do that
> in SVN? I understand that unlike git, a submitted branch is already set
> in stone? Please advise. This branch is my sole SVN experience in entire
> life, so I'm quite a newbie. I did google and ask another guy, though,
> to no avail.
Well, committed content is indeed "set in stone", so to speak (i.e.
"part of history"). You could of course start a new branch and redo
everything, but I wouldn't recommend it (that would force me / others
to review it all again). The usual practice, if there are remarks
about the content, is to simply add additional commits fixing the
problems / adding more code / ... So for changes to the contents of
the file, I suggest you perform further commits on the branch.
OTOH, _commit messages_ are editable in SVN. They are part of the so
called "revision properties" (contrary to "versioned properties"). So,
for remarks about the commit messages, the usual practice is to simply
edit them. For example with this command:
svn propedit --revprop -r$REV svn:log $URL
(the $URL can be any url pointing to the svn repository, since
revision numbers are global for the repo)
This will open the current log message in your $SVN_EDITOR (fallback
to $EDITOR).
> > I do get 8 warnings with this sort of message (always similar
> > stacktrace, always coming from UtilTests.testFileMerge, line 120):
> > [[[
> > WARNING: JNI local refs: 33, exceeds capacity: 32
> > at java.net.NetworkInterface.getAll(java.base@11.0.6/Native Method)
> > at
> java.net.NetworkInterface.getNetworkInterfaces(java.base@11.0.6/NetworkInterface.java:359)
>
> To my knowledge, this is an oversight in
> 'java.net.NetworkInterface.getAll' itself.
>
> JNI documentation says [1]:
> [[[
> You can call the JNI EnsureLocalCapacity() method to tell the JVM that
> you'll be using more than 16 local references. This allows the JVM to
> optimize the handling of local references for that native. Failure to
> inform the JVM can lead to a FatalError if the required local references
> cannot be created, or poor performance that's due to a mismatch between
> the local-reference management employed by the JVM and the number of
> local references used.
> ]]]
>
> I'm observing the same warnings in other Java programs which are
> unrelated to SVN. I think that this can be ignored for the purposes of
> this JavaHL related discussion.
>
> [1] https://developer.ibm.com/languages/java/articles/j-jni/
Yes, let's ignore that here ... it's unrelated.
--
Johan
Re: svn commit: r1882520 -
/subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
Posted by Alexandr Miloslavskiy <al...@syntevo.com>.
Thanks for reviewing! You're quite the savior of this branch :)
On 27.01.2021 11:02, Johan Corveleyn wrote:
> was wondering what you meant by "This error is easily seen
> when running JavaHL tests"
If you checkout r1882518, tests die with 'Bad global or local ref passed
to JNI' before this error can be seen. However, if you also cherry-pick
r1882522 on top to fix this problem, that's where you'll see 'JNI call
made with exception pending' that I fixed in r1882520.
Sorry for confusion. I edited/reordered patches a few times before
committing, and didn't notice that the commit message is not too good
with this ordering.
Which brings me to next question. In other replies, you suggested that I
make some edits in code and also amend commit messages. How do I do that
in SVN? I understand that unlike git, a submitted branch is already set
in stone? Please advise. This branch is my sole SVN experience in entire
life, so I'm quite a newbie. I did google and ask another guy, though,
to no avail.
> I do get 8 warnings with this sort of message (always similar
> stacktrace, always coming from UtilTests.testFileMerge, line 120):
> [[[
> WARNING: JNI local refs: 33, exceeds capacity: 32
> at java.net.NetworkInterface.getAll(java.base@11.0.6/Native Method)
> at
java.net.NetworkInterface.getNetworkInterfaces(java.base@11.0.6/NetworkInterface.java:359)
To my knowledge, this is an oversight in
'java.net.NetworkInterface.getAll' itself.
JNI documentation says [1]:
[[[
You can call the JNI EnsureLocalCapacity() method to tell the JVM that
you'll be using more than 16 local references. This allows the JVM to
optimize the handling of local references for that native. Failure to
inform the JVM can lead to a FatalError if the required local references
cannot be created, or poor performance that's due to a mismatch between
the local-reference management employed by the JVM and the number of
local references used.
]]]
I'm observing the same warnings in other Java programs which are
unrelated to SVN. I think that this can be ignored for the purposes of
this JavaHL related discussion.
[1] https://developer.ibm.com/languages/java/articles/j-jni/
Re: svn commit: r1882520 - /subversion/branches/javahl-1.14-fixes/subversion/bindings/javahl/native/JNIUtil.cpp
Posted by Johan Corveleyn <jc...@gmail.com>.
On Thu, Oct 15, 2020 at 12:14 PM <am...@apache.org> wrote:
>
> Author: amiloslavskiy
> Date: Thu Oct 15 10:14:30 2020
> New Revision: 1882520
>
> URL: http://svn.apache.org/viewvc?rev=1882520&view=rev
> Log:
> JavaHL: Fix 'JNI call made with exception pending' error
>
> It is incorrect to call Java method with 'CallObjectMethod' when there
> is an unhandled Java exception already pending. The fix is to
> temporarily move exception out of the way.
>
> This error is easily seen when running JavaHL tests.
Hi Alexandr,
I agree with this change, but was wondering what you meant by "This
error is easily seen when running JavaHL tests"? Is there a warning or
...? Can you explain how to see the effect of "the absence of this
fix"?
Same remark / question for your next commit, r1882521, where you say
in the commit message:
"These warnings are seen in JavaHL tests due to '-Xcheck:jni' option."
For context: I'm building on Windows, with AdoptOpenJDK-11.0.6.10. I
have performed a "release" build, but have adjusted win-tests.py to
pass the -Xcheck:jni option. I'm not seeing any warnings that point to
this, when running the javahl testsuite.
I am however seeing a bunch of other warnings, but I assume these are
totally unrelated to the work you did on the javahl-1.14-fixes branch:
I do get 8 warnings with this sort of message (always similar
stacktrace, always coming from UtilTests.testFileMerge, line 120):
[[[
WARNING: JNI local refs: 33, exceeds capacity: 32
at java.net.NetworkInterface.getAll(java.base@11.0.6/Native Method)
at java.net.NetworkInterface.getNetworkInterfaces(java.base@11.0.6/NetworkInterface.java:359)
at sun.security.provider.SeedGenerator.addNetworkAdapterInfo(java.base@11.0.6/SeedGenerator.java:229)
at sun.security.provider.SeedGenerator$1.run(java.base@11.0.6/SeedGenerator.java:179)
at sun.security.provider.SeedGenerator$1.run(java.base@11.0.6/SeedGenerator.java:167)
at java.security.AccessController.doPrivileged(java.base@11.0.6/Native
Method)
at sun.security.provider.SeedGenerator.getSystemEntropy(java.base@11.0.6/SeedGenerator.java:167)
at sun.security.provider.AbstractDrbg$SeederHolder.<clinit>(java.base@11.0.6/AbstractDrbg.java:551)
at sun.security.provider.AbstractDrbg.getEntropyInput(java.base@11.0.6/AbstractDrbg.java:505)
at sun.security.provider.AbstractDrbg.getEntropyInput(java.base@11.0.6/AbstractDrbg.java:494)
at sun.security.provider.AbstractDrbg.instantiateIfNecessary(java.base@11.0.6/AbstractDrbg.java:696)
- locked <0x000000008a727f88> (a sun.security.provider.HashDrbg)
at sun.security.provider.AbstractDrbg.engineNextBytes(java.base@11.0.6/AbstractDrbg.java:378)
at sun.security.provider.AbstractDrbg.engineNextBytes(java.base@11.0.6/AbstractDrbg.java:334)
at sun.security.provider.DRBG.engineNextBytes(java.base@11.0.6/DRBG.java:233)
at java.security.SecureRandom.nextBytes(java.base@11.0.6/SecureRandom.java:741)
at java.security.SecureRandom.next(java.base@11.0.6/SecureRandom.java:798)
at java.util.Random.nextLong(java.base@11.0.6/Random.java:424)
at java.io.File$TempDirectory.generateFile(java.base@11.0.6/File.java:1930)
at java.io.File.createTempFile(java.base@11.0.6/File.java:2078)
at org.apache.subversion.javahl.UtilTests.testFileMerge(UtilTests.java:120)
at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(java.base@11.0.6/Native
Method)
at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(java.base@11.0.6/NativeMethodAccessorImpl.java:62)
at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base@11.0.6/DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(java.base@11.0.6/Method.java:566)
at junit.framework.TestCase.runTest(TestCase.java:176)
at junit.framework.TestCase.runBare(TestCase.java:141)
at junit.framework.TestResult$1.protect(TestResult.java:122)
at junit.framework.TestResult.runProtected(TestResult.java:142)
at junit.framework.TestResult.run(TestResult.java:125)
at junit.framework.TestCase.run(TestCase.java:129)
at junit.framework.TestSuite.runTest(TestSuite.java:252)
at junit.framework.TestSuite.run(TestSuite.java:247)
at junit.framework.TestSuite.runTest(TestSuite.java:252)
at junit.framework.TestSuite.run(TestSuite.java:247)
at junit.textui.TestRunner.doRun(TestRunner.java:116)
at junit.textui.TestRunner.doRun(TestRunner.java:109)
at junit.textui.TestRunner.run(TestRunner.java:77)
at org.apache.subversion.javahl.RunTests.main(RunTests.java:119)
]]]
I suppose I have some lower default limit for these JNI local refs ...
perhaps this should be adjusted somewhere in our test suite (maybe
this was already done for unix, but not for Windows, but I haven't
investigated further)
Thanks,
--
Johan