Review Request JDK-8189193: FindClass mistakenly uses system class loader when the initiating loader is bootstrap loader

[mandy chung]

On 10/18/17 4:15 PM, coleen.phillimore at oracle.com wrote:
>
> Hi Mandy,
>
> In the wrong version, for the case where k->class_loader() is NULL at 
> line 419 or 423, would get reset to the system class loader?

Yes and that's the bug.

>
> I'm so sorry for the unhelpful help, I was hoping to make the logic 
> simpler but it backfired.
>

No worries.  I should have caught that myself sorry.

> The new versions logic is a lot more straightforward and nice.  It 
> takes some studying to determine that loader isn't a naked oop though 
> so it might be safer to make it a Handle at 403.

Let me take a look.

>   Or else:
>
> 428 Handle loader_h = Handle(THREAD, loader);
>
>
> could be
>
> 428 Handle loader_h(THREAD, loader);
>

Will change to the above.

>
> Looks good.  Sorry again.
> thanks for fixing it quickly.

Thanks for your prompt review.

Mandy

> Coleen
>
>
> On 10/18/17 5:52 PM, mandy chung wrote:
>> The bug creeped in when I improved the code to use loader.is_null() 
>> to set to system class loader per Coleen's suggestion.  It's my 
>> oversight of the null loader case.  The is the version I pushed.
>> http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8188052/webrev.02/
>>
>> The previous version was correct.
>> http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8188052/webrev.01/
>>
>> I didn't go back to webrev.01.  The fix is an improvement to it.
>>
>> Mandy
>>
>> On 10/18/17 2:48 PM, David Holmes wrote:
>>> Hi Mandy,
>>>
>>> Do you still have the webrev for 8188052? I need to see how this got 
>>> through given we looked at it so many times. :(
>>>
>>> Thanks,
>>> David
>>>
>>> On 19/10/2017 7:30 AM, mandy chung wrote:
>>>> This is a regression caused by JDK-8188052. FindClass should only 
>>>> use system class loader when there is no caller class or when it's 
>>>> called from JNI_OnUnload. When there is a caller class, it should 
>>>> either use its class loader or the one associated with JNI_Onload.
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8189193/webrev.00/
>>>>
>>>> JBS:https://bugs.openjdk.java.net/browse/JDK-8189193
>>>>
>>>> thanks
>>>> Mandy
>>
>