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

[serguei.spitsyn at 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.spitsyn at 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.spitsyn at oracle.com wrote:
>>> PING: This is pretty small and easy to review fix.
>>>
>>> Thanks!
>>> Serguei
>>>
>>>
>>> On 5/19/20 09:28, serguei.spitsyn at 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
>>>>
>>>>
>>>>
>>>
>