Review request (L) 8003419: NPG: Clean up metadata created during class loading if failure

[Jon Masamitsu]

Coleen,

http://cr.openjdk.java.net/~coleenp/8003419/src/share/vm/classfile/classFileParser.hpp.udiff.html

*+    // metadata created before the instance klass is created.  Must be*
*+   // deallocated if classfile parsing returns an error.*

Could this also have said

"Must be deallocated if not transferred to the InstanceKlass upon 
sucessful class loading
in which case these pointers have been set to NULL."

At first I didn't understand why the destructor didn't always deallocate the
metadata.

Can all the fields with pointers to metadata be grouped together
and in the destructor iterate through the pointers to see that they are
all NULL?  Worth doing to catch leaks?


This file just had so many changes in it I quickly bogged down. I'll
try again later.  So far only

http://cr.openjdk.java.net/~coleenp/8003419/src/share/vm/classfile/classFileParser.cpp.udiff.html

   2991    // Assign allocations if needed

allocations or annotations?

Can you make this unsigned?

3288       int map_count = _super_klass->nonstatic_oop_map_count();



Jon


On 3/5/2013 11:30 AM, Coleen Phillimore wrote:
>
> Resend to hotspot-dev.   I don't think anyone reads 
> hotspot-runtime-dev anymore.
> Please review!!
> Thanks,
> Coleen
>
> On 03/04/2013 03:13 PM, Coleen Phillimore wrote:
>> Summary: Store metadata on ClassFileParser instance to be cleaned up 
>> by destructor.  This enabled some refactoring of the enormous 
>> parseClassFile function.
>>
>> Note: I moved out code that lays out the java object into it's own 
>> function, but the diffs made it look like I made more than minor 
>> changes.  The changes I made was changing class_name to _class_name 
>> and cp to _cp, and not more than that.   To see these changes, you'll 
>> have to look at before/after code and not the diffs.  I also tried 
>> moving this layout_fields() function around to it wouldn't false 
>> match but I wanted to keep it next to parseClassFile() and not move 
>> it too far away.
>>
>> Tested with runThese jck, jck8 lang and vm, NSK vm.quick.testlist, 
>> java/lang/invoke jtreg tests and java/lang/instrument jtreg tests.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8003419/
>> bug link at http://bugs.sun.com/view_bug.do?bug_id=8003419
>>
>> Thanks,
>> Coleen
>>
>