RFR: 8210926: vmTestbase/nsk/jvmti/scenarios/allocation/AP11/ap11t001/TestDescription.java failed with JVMTI_ERROR_INVALID_CLASS in CDS mode
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Sun Sep 30 15:56:04 UTC 2018
This looks good, particularly the removal of unlink class. Thank you
for confirming where it's set back to loaded.
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,
> #1 SystemDictionary::define_instance_class (k=0x800006488,
> #2 SystemDictionary::find_or_define_instance_class
> (class_name=0x800419080, class_loader=..., k=0x800006488,
> #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=...,
> #5 SystemDictionary::resolve_instance_class_or_null_helper
> (class_name=0x800419080, class_loader=..., protection_domain=...,
> #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.
> 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?
>> On 9/28/18 4:43 PM, coleen.phillimore at oracle.com wrote:
>>> This looks good!
>>> 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:
>>>> 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.
More information about the hotspot-runtime-dev