On Tue, 18 Aug 2026 09:04:56 GMT, Markus Grönlund <[email protected]> wrote:
>> Have you confirmed that? It may be the order doesn't matter, but it seems an
>> unnecessary change to make unless you want to verify that.
>
> 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) {
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32218#discussion_r3802769738
