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

[Coleen Phillimore]

On 12/3/14, 2:31 PM, harold seigel wrote:
> 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));

Oh, this is my fault. https://bugs.openjdk.java.net/browse/JDK-8066624
If it looks inconsistent, maybe it can be changed to CHECK_(nullHandle);
>
>
> 2. In ~ClassFileParser(), does MetadataFactory::free_array() and 
> Annotations::free_contents() need to be called if the *_annotation 
> fields are NULL?

free_array() checks for null and is inlined so there isn't another need 
for checking if the Array is null first.

This change looks good to me.

Coleen
>
>
> 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
>