RFR 8028497: SIGSEGV at ClassLoaderData::oops_do(OopClosure*, KlassClosure*, bool)

[Coleen Phillimore]

I have a new version of this change based on code that Ioi noticed that 
I was missing for making restoring the method's adapters restartable if 
something fails during restore_unshareable_info, and the constant pool 
resolved references array.
Please review this new code (only method.* and instanceKlass.cpp and 
constantPool.cpp have differences).  Reran tests with CDS and CMS.

http://cr.openjdk.java.net/~coleenp/8028497_2/

Thanks,
Coleen

On 3/26/14 5:15 PM, Coleen Phillimore wrote:
>
> Hi Jon,
> Thank you for looking at the code changes.
>
> On 3/26/14 12:24 PM, Jon Masamitsu wrote:
>> Coleen,
>>
>> Did you remove Class_obj_allocate() because it was only
>> called in that one spot and  use obj_allocate() did most
>> of the work?
>
> I deleted it because GC doesn't need to know how these javaClasses are 
> set up and I spent way to long looking for this code and didn't find 
> it where it belongs, so I removed this.  This Class_obj_allocate is an 
> artifact of the permgen code.
>
>>
>> http://cr.openjdk.java.net/~coleenp/8028497/src/share/vm/oops/instanceMirrorKlass.cpp.frames.html 
>>
>>
>>  373   // the size in the mirror size itself.
>>
>> Drop the second "size".  You know how we GC guys are
>> getting with our comments :-).
>
> Yes, I noticed that.  Thanks - fixed.
>>
>> http://cr.openjdk.java.net/~coleenp/8028497/src/share/vm/oops/klass.cpp.frames.html 
>>
>>
>>  498 void Klass::restore_unshareable_info(TRAPS) {
>>  499   // If an exception happened during CDS restore, some of these 
>> fields may already be
>>  500   // set.  We leave the class on the CLD list, even if 
>> incomplete so that we don't
>>  501   // modify the CLD list outside a safepoint.
>>  502   if (class_loader_data() == NULL) {
>>  503     ClassLoaderData* loader_data = 
>> ClassLoaderData::the_null_class_loader_data();
>>
>> I can see that this works with the current CDS (sharing only classes 
>> loader
>> with the NULL class loader).  Will it work if the shared classes are 
>> loaded
>> with an application class loader?
>
> I hope Ioi can answer that.  I assume so because the classes don't 
> change class loaders if restoring shareable info fails.
>
> Ioi pointed out to me (not on open list) that I missed making 
> link_method() and creating the constant pool resolved references array 
> restartable, so I'm testing a fix for that also and I'll post an 
> update later.   But you've seen most of it.
>
> Thanks!!
> Coleen
>>
>> Rest looks good.
>>
>> Jon
>>
>> On 3/25/2014 1:42 PM, Coleen Phillimore wrote:
>>> Summary: Keep class in CLD::_klasses list and mirror created for CDS 
>>> classes if OOM during restore_shareable_info(). This keeps pointers 
>>> consistent for CMS.
>>>
>>> This change makes restoring the mirror for shared classes 
>>> restartable if there is an OOM in the middle of 
>>> restore_unshareable_info.   The mirror field in the classes and CLD 
>>> link is not cleaned up anymore.  The class isn't marked loaded 
>>> though so must be loaded again.
>>>
>>> Tested with Stefan's test case with hard coded OOM in jvm, so can't 
>>> add the test.   Also ran all the tests against -client and server 
>>> with -Xshare:auto and with -XX:+UseConcMarkSweepGC.
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8028497/
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8028497
>>>
>>> Thanks,
>>> Coleen
>>
>