RFR (S) JDK-8007725 - Klass::restore_unshareable_info() triggers assert(k->java_mirror() == NULL)

[Coleen Phillimore]

The assert string could be a comment about the condition, and you can 
have the comment above the assert.  I prefer 1 line to the 4 lines.

Coleen

On 03/14/2013 12:04 PM, Ioi Lam wrote:
> Hi Harold,
>
> I used the "if" so that it would be easier to comment about the condition.
>
> In product build, the "if" block would be eliminated by the compiler. 
> I verified that in the Linux build.
>
> Thanks
> - Ioi
>
> On 03/14/2013 06:58 AM, harold seigel wrote:
>> Hi Ioi,
>>
>> You could simplify this in method.cpp from:
>>
>>     + if (!DumpSharedSpaces) {
>>     +   // _method_data may be NULL if link_method failed due to OOM.
>>     + } else {
>>          assert(_method_data == NULL, "unexpected method data?");
>>     + }
>>     set_method_data(NULL);
>>
>> to something like:
>>
>>     assert(!DumpSharedSpaces || _method_data == NULL, "unexpected
>>     method data?");
>>     set_method_data(NULL);
>>
>> Then the DumpSharedSpaces flag is only checked if the assert is 
>> processed.
>>
>> Harold
>>
>> On 3/14/2013 9:46 AM, Coleen Phillimore wrote:
>>>
>>> This looks good.   Thank you for fixing it.
>>> Coleen
>>>
>>> On 03/14/2013 01:49 AM, Ioi Lam wrote:
>>>> Hi, could someone review this? This fixes a crash related to OOM 
>>>> handling.
>>>>
>>>> Thanks
>>>> - Ioi
>>>>
>>>> On 03/11/2013 12:47 PM, Ioi Lam wrote:
>>>>> Please review:
>>>>> http://cr.openjdk.java.net/~iklam/8007725/null_mirror_001/
>>>>>
>>>>> Bug: NPG: Klass::restore_unshareable_info() triggers
>>>>>     assert(k->java_mirror() == NULL)
>>>>>
>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8007725
>>>>>  or https://jbs.oracle.com/bugs/browse/JDK-8007725
>>>>>
>>>>>     (The bugs.sun.com page may be unavailable for a couple of days 
>>>>> due
>>>>>      to recent change of confidentiality status.)
>>>>>
>>>>> Summary of fix:
>>>>>
>>>>>     The assert is at:
>>>>>
>>>>>     --------
>>>>>     # Internal Error (src/share/vm/classfile/javaClasses.cpp:527)
>>>>>     # assert(k->java_mirror() == NULL) failed: should only assign 
>>>>> mirror once
>>>>>     java_lang_Class::create_mirror(KlassHandle)
>>>>>     Klass::restore_unshareable_info()
>>>>>     InstanceKlass::restore_unshareable_info()
>>>>>     instanceKlassHandle 
>>>>> SystemDictionary::load_shared_class(instanceKlassHandle,Handle)
>>>>>     --------
>>>>>
>>>>>     This happens if during class loading, an OOM happened after
>>>>>     Klass::restore_unshareable_info() has called
>>>>>     java_lang_Class::create_mirror(), but before the class loading 
>>>>> is completed.
>>>>>
>>>>>     At this point, the class is not in the system dictionary. 
>>>>> However the
>>>>>     Klass structure is partially initialized.
>>>>>
>>>>>     If the app tries to load the same class again, the assert 
>>>>> happens.
>>>>>
>>>>>     The fix is to undo all work performed by
>>>>>     InstanceKlass::restore_unshareable_info().
>>>>>
>>>>> Tests executed:
>>>>>
>>>>>     * JPRT
>>>>>     * JCK (vm, lang, api/signaturetest)
>>>>>     * UTE (vm.quick.testlist, )
>>>>>
>>>>> Thanks,
>>>>> Ioi
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130314/b3dfc697/attachment.html