RFR: 8065634: Crash in InstanceKlass::clean_method_data when _method is NULL
harold.seigel at oracle.com
Fri Dec 5 14:04:27 UTC 2014
Thanks for restoring the 'else' and the 'nullHandle'. It looks good!
On 12/5/2014 8:51 AM, Stefan Karlsson wrote:
> Hi Harold,
> Thanks for looking at the patch. My first patch used nullHandle and
> did the freeing was done inside an else statement, but during a
> pre-review of the patch I got feedback to change it to CHECK_NULL and
> get rid of the else statement. :)
> Here's a new webrev with nullHandle and the else statement:
> On 2014-12-03 14:31, 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,
>> 4024 // Finalize the Annotations metadata object,
>> 4025 // now that all annotation arrays have been created.
>> 4026 create_combined_annotations(CHECK_NULL);
>> 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
>>> 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.
>>> JPRT, parallel_class_loading testlist, Aurora adhoc run
More information about the hotspot-dev