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

[mandy chung]

On 10/5/17 1:13 PM, coleen.phillimore at oracle.com wrote:
>
> 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.
>>

That's from existing code which may not be touched for a long time. 
Since I'm on this file, I can clean this up.
>> 407     if (loader.is_null() &&
>>

- if (loader.is_null() &&
+ if (k->class_loader() == NULL && Already fixed per David's comment.

>> 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?
>>

This is existing code.   I removed these lines and pass CHECK_NULL to 
call_static.

How does this version look?
http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8188052/webrev.02/src/hotspot/share/prims/jni.cpp.sdiff.html

One other thing I notice: it only sets the protection_domain to the 
caller class when FindClass is called from JNI_OnLoad/JNI_OnUnload.

For other caller class, it uses null protection_domain. I would expect 
it should use the caller's protection domain  to look up a class.  I 
think it's a bug, isn't?

Mandy