RFR: 8210926: vmTestbase/nsk/jvmti/scenarios/allocation/AP11/ap11t001/TestDescription.java failed with JVMTI_ERROR_INVALID_CLASS in CDS mode
Jiangli Zhou
jiangli.zhou at oracle.com
Sat Sep 29 23:06:49 UTC 2018
Hi David,
Thanks a lot for the review! Here is the updated webrev:
http://cr.openjdk.java.net/~jiangli/8210926/webrev.01/.
On 9/28/18 9:23 AM, David Holmes wrote:
> Hi Jiangli,
>
> This seems a little odd:
>
> 2303 // Unlink the class
> 2304 if (is_linked()) {
> 2305 unlink_class();
> 2306 }
> 2307
> 2308 _init_state = allocated;
>
> unlink_class() will set the _init_state to loaded then we set it back
> to allocated.
Thanks for pointing that out. I was not too happy about that either. The
only usage of unlink_class() (which just resets the state back to
'loaded') is from InstanceKlass::remove_unshareable_info(). I've updated
the webrev to remove unlink_class(). Hope that makes the code a little
more cleaner.
>
> There's a typo in a comment "the the".
Fixed.
>
> Thanks,
> David
> (Literally running out the door :) )
Hope your flight was enjoyable!
Thanks,
Jiangli
>
> On 28/09/2018 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
>>
>>
More information about the hotspot-runtime-dev
mailing list