On Tue, 18 Aug 2026 09:25:12 GMT, Markus Grönlund <[email protected]> wrote:
>> Because this code lacks asserts, it is impossible to verify.
>>
>> My argument is that it is wrong to publish and share a klass, using a
>> Dictionary, that is not in a "loaded" state. The comment (again,
>> unfortunately no asserts), in Dictionary::add_klass() says as much, although
>> I grant "loaded class" could be interpreted non-technically:
>>
>> // Add a loaded class to the dictionary.
>> void Dictionary::add_klass(JavaThread* current, Symbol* class_name,
>> InstanceKlass* klass) {
>>
>> Ideally, I would want the same triple repeated at all definition sites, if
>> possible:
>>
>> 1. JFR_ONLY(Jfr::on_definition(klass)) // has strong asserts that
>> !klass->is_loaded()
>> 2. klass->add_to_hierarchy(THREAD); assert(klass->is_loaded(), "Must be in
>> at least loaded state"); // this is what makes a klass go to state "loaded"
>> 3. dictionary->add_klass(THREAD, klass->name(), klass); // The klass is now
>> in "loaded" state and is published.
>
> More non-asserts:
>
> // Update hierarchy. This is done before the new klass has been added to the
> SystemDictionary. The Compile_lock
> // is grabbed, to ensure that the compiler is not using the class hierarchy.
> void InstanceKlass::add_to_hierarchy(JavaThread* current) {
The usual path calls add_to_hierarchy() first so this should be okay. If it
mattered, the usual case in system dictionary would more likely expose any
dependencies on these states. I think this is okay.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32218#discussion_r3845054615
