RFR: 8210926: vmTestbase/nsk/jvmti/scenarios/allocation/AP11/ap11t001/TestDescription.java failed with JVMTI_ERROR_INVALID_CLASS in CDS mode

[Jiangli Zhou]

Thanks, Coleen!

Jiangli


On 9/30/18 8:56 AM, coleen.phillimore at oracle.com wrote:
>
> This looks good, particularly the removal of unlink class.  Thank you 
> for confirming where it's set back to loaded.
> Coleen
>
>
> On 9/29/18 7:32 PM, Jiangli Zhou wrote:
>> Hi Coleen,
>>
>> Thanks a lot for the review! A shared class is set to 'loaded' by 
>> SystemDictionary::add_to_hierarchy() via the same call path for both 
>> dynamically loaded class and shared classes. If we set 'loaded' state 
>> in restore_unshareable_info, we then would need to add CDS specifics 
>> to SystemDictionary::add_to_hierarchy to avoid resetting it.
>>
>> #0  SystemDictionary::add_to_hierarchy (k=0x800006488, 
>> __the_thread__=0x7ffff001b000)
>> #1  SystemDictionary::define_instance_class (k=0x800006488, 
>> __the_thread__=0x7ffff001b000)
>> #2  SystemDictionary::find_or_define_instance_class 
>> (class_name=0x800419080, class_loader=..., k=0x800006488, 
>> __the_thread__=0x7ffff001b000)
>> #3  SystemDictionary::load_instance_class (class_name=0x800419080, 
>> class_loader=..., __the_thread__=0x7ffff001b000)
>> #4  SystemDictionary::resolve_instance_class_or_null 
>> (name=0x800419080, class_loader=..., protection_domain=..., 
>> __the_thread__=0x7ffff001b000)
>> #5  SystemDictionary::resolve_instance_class_or_null_helper 
>> (class_name=0x800419080, class_loader=..., protection_domain=..., 
>> __the_thread__=0x7ffff001b000)
>> #6  SystemDictionary::resolve_or_null (class_name=0x800419080, 
>> class_loader=..., protection_domain=..., __the_thread__=0x7ffff001b000)
>>
>> ...
>>
>> I updated the webrev: 
>> http://cr.openjdk.java.net/~jiangli/8210926/webrev.01/ to remove 
>> unlink_class() and added more comments. Please let me know if that 
>> looks okay to you.
>>
>> Thanks,
>> Jiangli
>>
>> On 9/28/18 1:58 PM, coleen.phillimore at oracle.com wrote:
>>>
>>> On second thought, where does this set the shared class to loaded? 
>>> Should that be done in restore_unshareable_info?
>>> thanks,
>>> Coleen
>>>
>>> On 9/28/18 4:43 PM, coleen.phillimore at oracle.com wrote:
>>>> This looks good!
>>>> Coleen
>>>>
>>>> On 9/28/18 10:08 AM, Jiangli Zhou wrote:
>>>>> Please review the fix for JDK-8210926. This is a bug in the CDS 
>>>>> code that's exposed by JvmtiEnv::GetLoadedClasses(), and can be 
>>>>> manifested in different failures with the following tests:
>>>>>
>>>>>   com/sun/jdi/ClassesByName2Test.java
>>>>> vmTestbase/nsk/jvmti/scenarios/allocation/AP11/ap11t001/TestDescription.java 
>>>>>
>>>>>   runtime/RedefineTests/ModifyAnonymous.java
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~jiangli/8210926/webrev.00/
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8210926
>>>>>
>>>>> SystemDictionary::add_to_hierarchy() sets a InstanceKlass 
>>>>> init_state to ‘loaded’ right before it is added to the the 
>>>>> SystemDictionary. JvmtiEnv::GetLoadedClasses() retrieves loaded 
>>>>> classes’ (InstanceKlasses in 'loaded' state and arrays) mirrors 
>>>>> (Class objects). At CDS dump time, a InstanceKlass::_init_state is 
>>>>> reset back to 'loaded' state before writing out the archived data. 
>>>>> At runtime during loading of a shared class, there is a 'brief' 
>>>>> moment JvmtiEnv::GetLoadedClasses() in another thread could see a 
>>>>> shared class in ‘loaded’ state without mirror. NULL mirror is not 
>>>>> the only issue, other fields of the shared InstanceKlass may not 
>>>>> be setup properly before SystemDictionary::add_to_hierarchy(). To 
>>>>> fix the issue, we need to reset to _init_state to 'allocated' 
>>>>> state before writing out the archived classes at dump time.
>>>>>
>>>>> Verified the fix with running ClassesByName2Test.java using mach5 
>>>>> (thanks Chris for providing the reproducible case). Tested with 
>>>>> tier1 - tier3 in both default CDS mode and no CDS mode.
>>>>>
>>>>> Thanks,
>>>>> Jiangli
>>>>>
>>>>>
>>>>
>>>
>>
>