RFR: 8364111: InstanceMirrorKlass iterators should handle CDS and hidden classes consistently

[Coleen Phillimore]

On Fri, 25 Jul 2025 15:31:48 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> See the bug for more investigation. I think the root cause is already fixed in one of the `InstanceMirrorKlass` iterators, but not in the other one. Additionally, only one iterator handles the hidden classes. Also, there is a stale comment about CMS that apparently prevents us from asserting `is_primitive`, which is not relevant anymore.
>> 
>> So this PR commons out the metadata handling on both iterators, which fixes the GenShen+CDS bug, likely fixes hidden classes bug somewhere, and prevents the omission like this from happening again.
>> 
>> Additional testing:
>>  - [x] Linux x86_64 server fastdebug, Generational Shenandoah + CDS bugs are not reproducing now
>>  - [x] Linux x86_64 server fastdebug, `tier1`
>>  - [ ] Linux x86_64 server fastdebug, `all`
>
> src/hotspot/share/oops/instanceMirrorKlass.inline.hpp line 69:
> 
>> 67:     // Klass is null means this has been a mirror for a primitive type
>> 68:     // that we do not need to follow as they are always strong roots.
>> 69:     assert(java_lang_Class::is_primitive(obj), "Sanity");
> 
> Question, since this assert is new:
> 
> When we create a mirror (`java_lang_Class::allocate_mirror`) during normal - non-shared - class loading, is there not a small time window where the mirror exists on the heap, but the Klass is still NULL? Could concurrent heap walkers not observe that?

Good point.  We create the mirror then initialize the class, so a concurrent GC could see a null Klass pointer.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26477#discussion_r2231567610