RFR 8217846: [Graal] vmTestbase/nsk/jdi/VirtualMachine/instanceCounts/instancecounts003/instancecounts003.java crash

[Lois Foltan]

On 5/8/2019 8:33 AM, Harold Seigel wrote:
> Hi Lois,
>
> Thanks for the review!
>
> It is convenient to leave the calls to add_class() where they are 
> because the loader_data is already available in those functions. 
> Moving the calls to add_class() to complete_create_array_klass() would 
> require extracting the loader_data in that function or passing it as 
> an additional parameter.

Hi Harold,

I'm okay with whatever you decide.

My reasoning was that we could eliminate 2 spots where we have to 
remember to add the class to the loader's  classLoaderData list down to 
1 that covers both.  I think the loader_data is available within 
ArrayKlass::complete_create_array_klass() via the ArrayKlass itself, 
k->class_loader_data(), so I don't think you would have to pass it in.

Thanks,
Lois

>
> Thanks, Harold
>
> On 5/7/2019 3:27 PM, Lois Foltan wrote:
>> On 5/7/2019 1:05 PM, Harold Seigel wrote:
>>> Hi,
>>>
>>> Please review this fix for JDK-8217846.  The fix adds array classes 
>>> to their classLoaderData list after creating the class's mirror 
>>> because if mirror creation fails (perhaps because of an OOME) then 
>>> the class should not be added to its classLoaderData list.
>>>
>>> Open Webrev: http://cr.openjdk.java.net/~hseigel/bug_8217846/webrev/
>>>
>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8217846
>>>
>>> The fix was tested by running the test over 1000 times.
>>>
>>> The fix was regression tested by running Mach5 tiers 1 and 2 tests 
>>> and builds on Linux-x64, Solaris, Windows, and Mac OS X and by 
>>> running Mach5 tiers 3-7 tests on Linux-x64.
>>>
>>> Thanks, Harold
>>>
>>
>> Hi Harold,
>>
>> This looks good.  Would you consider moving the code to add the array 
>> class to the its class loader list into 
>> ArrayKlass::complete_create_array_klass as the last step, post mirror 
>> creation?
>>
>> Thanks,
>> Lois