Review Request JDK-8188052: JNI_FindClass needs to specify the class loading context used for library lifecycle hooks

[coleen.phillimore at oracle.com]

I should say, besides these style comments this change looks good.
Coleen

On 10/5/17 3:28 PM, coleen.phillimore at oracle.com wrote:
>
> http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8188052/webrev.00/src/hotspot/share/prims/jni.cpp.udiff.html 
>
>
> The type Klass has a class_loader() and protection_domain() accessors 
> so it is unnecessary to cast to InstanceKlass.  We went through and 
> removed many of these unnecessary InstanceKlass::casts.
>
> 407     if (loader.is_null() &&
>
> I agree with David's comment that this line will always evaluate to 
> true so should be cleaned up also.
>
> 413                              thread);
>  414       if (HAS_PENDING_EXCEPTION) {
>  415         Handle ex(thread, thread->pending_exception());
>  416         CLEAR_PENDING_EXCEPTION;
>  417         THROW_HANDLE_0(ex);
>  418       }
>
>
> This is strange too.  Why doesn't this just pass CHECK_NULL at line 
> 413, which is the same thing as checking for and clearing and throwing 
> the pending exception?
>
> Thanks,
> Coleen
>
>
> On 10/4/17 2:12 PM, mandy chung wrote:
>> This patch separates the JNI `FindClass` issue from the review thread 
>> for JDK-8188052 [1] into a different issue.
>>
>> webrev: 
>> http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8188052/webrev.00/index.html
>>
>> This patch changes `FindClass` to specify the class loading context 
>> used for the load and unload hook. `FindClass` when called from 
>> JNI_OnUnload is changed to use the system class loader as the 
>> context. This is an incompatible behavioral change and this bumps the 
>> version so that a native library can determine the new behavior if 
>> desire.  This change proposes to defines JNI_VERSION_10 and I expect 
>> this name will be revisited along with the version string for the 
>> proposed new release cadence [2].
>>
>> CSR:  https://bugs.openjdk.java.net/browse/JDK-8188069
>>
>> Thanks David for the suggested spec wordings.
>>
>> thanks
>> Mandy
>> [1] 
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-September/049292.html
>> [2] 
>> http://mail.openjdk.java.net/pipermail/discuss/2017-September/004281.html
>