RFR: 8065634: Crash in InstanceKlass::clean_method_data when _method is NULL

[harold seigel]

Hi Stefan,

The change looks good but I have a couple of questions.

1. Why does the call to create_combined_annotations(), in 
parseClassFile(), use CHECK_NULL when other calls use CHECK_(nullHandle) ?

4021     ClassAnnotationCollector parsed_annotations;
4022     parse_classfile_attributes(&parsed_annotations, CHECK_(nullHandle));
4023
4024     // Finalize the Annotations metadata object,
4025     // now that all annotation arrays have been created.
4026     create_combined_annotations(CHECK_NULL);
4027
4028     // Make sure this is the end of class file stream
4029     guarantee_property(cfs->at_eos(), "Extra bytes at the end of class file %s", CHECK_(nullHandle));


2. In ~ClassFileParser(), does MetadataFactory::free_array() and 
Annotations::free_contents() need to be called if the *_annotation 
fields are NULL?


Thanks, Harold

On 12/3/2014 9:21 AM, Stefan Karlsson wrote:
> Hi all,
>
> Please, review this patch to fix an issue where the GC encounters an 
> InstanceKlass before it has come far enough in the class loading process.
>
> Some background to the fix:
>
> When the JVM loads classes it has to keep track of the allocated 
> Metadata, and in the event of an exception deallocate the Metadata. 
> The ClassFileParser saves the Metadata pointers so that it knows what 
> needs to be deallocated. When the class loading has come far enough 
> there's a point were the GC can take over the ownership of the 
> Metadata and do deallocation of the both the InstanceKlass and the 
> Metadata, if needed. At that point the Metadata pointers in the 
> ClassFileParser are cleared and transfered over to the InstanceKlass.
>
> Most Metadata allocations happen before the InstanceKlass is 
> allocated, but the Annotations are allocated after the InstanceKlass 
> but before the transferal and clearing of the Metadata pointers in the 
> ClassFileParser. If the GC stops when allocating the Annotations, it 
> will likely find this InstanceKlass and assume that the Metadata are 
> correctly setup. This is exactly what's happening in the bug report. 
> The GC finds that the _methods array, unexpectedly, points to NULL.
>
> The proposed solution to this problem is to allocate the Annotations 
> before the InstanceKlass is allocated.
>
> http://cr.openjdk.java.net/~stefank/8065634/webrev.01
> https://bugs.openjdk.java.net/browse/JDK-8065634
>
> Testing:
> JPRT, parallel_class_loading testlist, Aurora adhoc run
>
> Thanks,
> StefanK