Re: PING: Re: RFR (XS): 8244571: assert(!_thread->is_pending_jni_exception_check()) failed: Pending JNI Exception Check during class loading

[serguei.spit...@oracle.com]

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