RFR: 8368727: CDS custom loader support causes asserts during class unloading

[David Holmes]

On Mon, 29 Sep 2025 02:20:34 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> When loading a class `k` from the CDS archive on behalf of a custom class loader, we were calling `loader_data->add_class(k)` too early. If the loading of `k` fails, it may be in `loader_data->_klasses`, but `k->init_state()` will remain `allocated`. This causes an assert during class unloading:
>> 
>> 
>> # assert(ik->is_loaded()) failed: class should be loaded 0x000000000b518eb8
>> V [libjvm.so+0xfa6bd5] InstanceKlass::unload_class(InstanceKlass*)+0x555 (instanceKlass.cpp:2870)
>> V [libjvm.so+0xa790e3] ClassLoaderData::classes_do(void (*)(InstanceKlass*))+0xc3 (classLoaderData.cpp:441
>> 
>> 
>> The fix is to move the  `loader_data->add_class(k)` call to `k->Klass::restore_unshareable_info()`. This is the same location as if `k` were loaded by the 3 built-in class loaders.
>> 
>> This was discovered when running some JCK tests in AOT mode. I've added a reproducer as a jtreg test case.
>
> src/hotspot/share/classfile/systemDictionaryShared.cpp line 177:
> 
>> 175:   // No longer holding SharedDictionary_lock
>> 176:   // No need to lock, as <ik> can be held only by a single thread.
>> 177:   loader_data->add_class(ik);
> 
> Are the comments pertaining to the `add_class` call?

Why not just move this to after the successful `load_shared_class` below? It is very hard to see the complete call sequence to when `restore_unshareable_info` gets called, to know that moving it there makes sense.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27511#discussion_r2386544451