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

[Aleksey Shipilev]

On Fri, 25 Jul 2025 16:31:16 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

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

Actually, I now see this assert is tautological: `java_lang_Class::is_primitive` checks that `java_class->metadata_field(_klass_offset) == nullptr`, which is the same as pulling the `Klass*` with `java_lang_Class::as_Klass` and null-checking it, like _we already just did_. So really the assert is redundant and we can purge it.

But this also suggests that whatever interaction with GC exists, it affects current code as well. I think it works with concurrent GCs through a different mechanism: the classes loaded during the GC and their mirrors would be either treated as implicitly live (e.g. via SATB invariants), or need to be specially keep-alived (Shenandoah's IU mode did this, IIRC).

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

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