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

[Jiangli Zhou]

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
>>>
>>>
>>
>