Hi Serguei,
Looks good, and I agree with David's comments. I was thinking the same
thing when I first looked at your original changes.
thanks,
Chris
On 5/22/20 2:32 AM, serguei.spit...@oracle.com wrote:
Hi David,
The updated webrev is with your comments addressed:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/8244571-jvmti-test-jnicheck.2/
Thanks,
Serguei
On 5/22/20 00:43, serguei.spit...@oracle.com wrote:
Hi David,
Thank you for the comments!
On 5/21/20 23:58, David Holmes wrote:
Hi Serguei,
On 22/05/2020 4:17 pm, serguei.spit...@oracle.com wrote:
PING: This is pretty small and easy to review fix.
Thanks!
Serguei
On 5/19/20 09:28, serguei.spit...@oracle.com wrote:
Please, review fix for:
https://bugs.openjdk.java.net/browse/JDK-8244571
Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/8244571-jvmti-test-jnicheck.1/
Summary:
There are two places in the native part of test that cause
assert and WARNING with the -Xcheck:jni.
The assert is because there is no check for pending exception
after the call to:
jni->CallBooleanMethod(klass, is_hid_mid);
Using a JNI ExceptionCheck()after the call fixes the issue.
bool res = jni->CallBooleanMethod(klass, is_hid_mid);
if (jni->ExceptionCheck()) {
LOG0("is_hidden: Exception in jni CallBooleanMethod\n");
}
return res;
That will fix the pending_jni_exception_check error, but if an
exception actually occurs what will be returned? And whatever is
returned, the callers of this method don't themselves check for
pending exceptions so they will treat it as if the exception didn't
occur - at least until we finally return to Java code. Perhaps any
exception should result in jni->FatalError as happens with any JVMTI
error?
You are right, it would be more clean to call jni->FatalError.
I was also thinking about it but also worried to get the exception
details.
The exception can be printed before call to FatalError.
The following call to the JVM TI function:
err = jvmti->GetClassLoaderClasses(loader, &count, &loader_classes);
produces the warning (with a java level stack trace): WARNING:
JNI local refs: 94, exceeds capacity: 32
It is because the GetClassLoaderClasses returns an array of
local references to the loader classes.
Using a JNI EnsureLocalCapacity() before the JVM TI call also
fixes the issue.
The warning suggests using 1024 is a bit of overkill. :)
What capacity would be more reasonable, 256 or 512?
Let's pick 256. This is just a warning, the test is still passing.
Thanks!
Serguei
Cheers,
David
Testing:
Running the test
test/hotspot/jtreg/serviceability/jvmti/HiddenClass locally.
Will run a mach5 job as well.
Thanks,
Serguei
