RFR: 8135322: ConstantPool::release_C_heap_structures not run in some circumstances

[Coleen Phillimore]

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
>