RFR(S): 8200466: Revisit the setting of _transitive_interfaces in InstanceKlass

[coleen.phillimore at oracle.com]

This sharing of metadata has been prone to bugs.  Can you have the same 
bug with the _secondary_super field? 
InstanceKlass::deallocate_contents() checks:

       secondary_supers() != transitive_interfaces() &&

and will free the field possibly from the super class if 
transitive_interfaces() is still NULL.  (If I understand this correctly).

Wouldn't it be easier just to eagerly set the super class, and keep the 
call to initialize_supers() where it is?

http://cr.openjdk.java.net/~ccheung/8200466/webrev.00/src/hotspot/share/classfile/classFileParser.cpp.udiff.html

**** 5884,5893 *****
*--- 5889,5899 ----*
*Annotations::free_contents(_loader_data, _fields_annotations);*
*Annotations::free_contents(_loader_data, _fields_type_annotations);*
*}*
**
*clear_class_metadata();*
*+ _transitive_interfaces = NULL;*
**

clear_class_metadata() already sets the _transitive_interfaces to NULL, 
so this shouldn't be needed.

thanks,
Coleen

On 4/27/18 12:46 AM, Calvin Cheung wrote:
> JBS: https://bugs.openjdk.java.net/browse/JDK-8200466
>
> webrev: http://cr.openjdk.java.net/~ccheung/8200466/webrev.00/
>
> This bug was discovered during the investigation of JDK-8200078 
> <https://bugs.openjdk.java.net/browse/JDK-8200078>.
>
> I've included the following comment in classFileParser.cpp to 
> summarize the change:
>
> // Delay the setting of _transitive_interfaces until after 
> initialize_supers() in
> // fill_instance_klass(). It is because the _transitive_interfaces may 
> be shared with
> // its _super. If an OOM occurs while loading the current klass, its 
> _super field
> // may not have been set. When GC tries to free the klass, the 
> _transitive_interfaces
> // may be deallocated mistakenly in 
> InstanceKlass::deallocate_interfaces(). Subsequent
> // dereferences to the deallocated _transitive_interfaces will result 
> in a crash.
>
> Testing: (on Oracle platforms)
>     hs-tier{1,2,3}
>     closed (soon will be open) PCL (parallel class loading) tests
>
> thanks,
> Calvin