RFR: 8135322: ConstantPool::release_C_heap_structures not run in some circumstances
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Thu Mar 31 03:02:07 UTC 2016
On 3/30/16 19:52, serguei.spitsyn at oracle.com wrote:
> Hi Andreas,
>
>
> One simple question so far:
> Should the _deallocate_list be set to NULL after deletion at L428:
> 424 ClassLoaderData::~ClassLoaderData() {
> 425 // Delete free list
> 426 if (_deallocate_list != NULL) {
> 427 free_deallocate_list();
> 428 delete _deallocate_list;
> 429 }
> 430
Please, skip my question.
It is too paranoid to do so. :)
Thanks,
Serguei
>
>
> Thanks,
> Serguei
>
>
> On 3/30/16 16:06, Coleen Phillimore wrote:
>>
>> Andreas,
>>
>> Good find! This seems like a safe fix. It's unfortunate to have to
>> walk the deallocate list, but I think it's short enough and less work
>> in comparison to having to call free_C_heap_structures on all the
>> other classes in the dead class loader data. And
>> deallocate_contents removes the scratch_class from the CLD _klasses
>> list so won't walk it twice.
>>
>> This is really good.
>>
>> Coleen
>>
>> On 3/30/16 8:12 AM, Andreas Eriksson wrote:
>>> Hi,
>>>
>>> Please review this fix for 8135322:
>>> ConstantPool::release_C_heap_structures not run in some circumstances.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8135322
>>> Webrev: http://cr.openjdk.java.net/~aeriksso/8135322/webrev.00
>>>
>>> The way this occurs is:
>>>
>>> 1. A ConstantPool, /merge_cp/, without a corresponding InstanceKlass is
>>> created during class redefinition.
>>> 2. /Merge_cp/ is added to ClassLoaderData::_deallocate_list, but is not
>>> cleaned up by ClassLoaderDataGraph::do_unloading because the holding
>>> ClassLoaderData is already dead.
>>> 3. ClassLoaderData::~ClassLoaderData is called, which usually runs
>>> InstanceKlass::release_C_heap_structures, which in turn will call
>>> ConstantPool::release_C_heap_structures. But since there is no
>>> corresponding InstanceKlass for /merge_cp/ it will never release its
>>> C heap structures.
>>> 4. For JDK 8 this means we leak a Monitor (ConstantPool::_lock). For
>>> JDK 9 the Monitor has been removed, but we will miss doing a call to
>>> ConstantPool::unreference_symbols, which is probably not good
>>> either.
>>>
>>>
>>> This change adds a call to free everything in the _deallocate_list
>>> first thing in ClassLoaderData::~ClassLoaderData.
>>>
>>> Note:
>>> Running deallocate_contents() on everything in the list might be
>>> doing unnecessary work:
>>> What we really would like to do is run release_C_heap_structures()
>>> on constant pools in the list that do not have a corresponding
>>> InstanceKlass. Not sure it's worth the effort to do the filtering,
>>> since I believe the _deallocate_list is usually not that long.
>>>
>>> Note 2:
>>> After fixing in JDK 9 I'll backport this to JDK 8, where it has a
>>> more immediately visible effect.
>>>
>>> Regards,
>>> Andreas
>>>
>>
>
More information about the hotspot-runtime-dev
mailing list