On Mon, 18 Aug 2025 20:00:40 GMT, Dan Heidinga <heidi...@openjdk.org> wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> @DanHeidinga comments
>
> src/hotspot/share/cds/aotLinkedClassBulkLoader.cpp line 127:
>
>> 125: precond(ik->local_interfaces()->at(i)->is_loaded());
>> 126: }
>> 127: });
>
> This debugging check is duplicated in `SystemDictionary::preload_class` in
> the `#ifdef ASSERT` block.
I removed this check.
> src/hotspot/share/classfile/javaClasses.cpp line 1017:
>
>> 1015: void java_lang_Class::set_mirror_module_field(JavaThread* current,
>> Klass* k, Handle mirror, Handle module) {
>> 1016: if (CDSConfig::is_using_preloaded_classes()) {
>> 1017: oop archived_module = java_lang_Class:: module(mirror());
>
> Suggestion:
>
> oop archived_module = java_lang_Class::module(mirror());
Fixed.
> src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 407:
>
>> 405: static class Holder {
>> 406: // All native libraries we've loaded.
>> 407: private static final Set<String> loadedLibraryNames =
>
> I'm concerned about this causing problems if we ever AOT initialize a user
> class that calls `System.loadLibrary` from its static initializer.
>
> Should we add a check to the `clear()` method above to ensure that only the
> expected libraries have been loaded during the assembly phase?
The assembly phase shouldn't execute any user Java code. If that happens, it
can result in many undesirable side effects. Loading user native library will
be just one example.
I think we need a stronger check -- make sure that no user classes are
initialized at the end of the assembly phase. This check should be sufficient
because executing code in any class will result in its initialization.
Maybe I should do that in a separate RFE?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26375#discussion_r2292686203
PR Review Comment: https://git.openjdk.org/jdk/pull/26375#discussion_r2292686311
PR Review Comment: https://git.openjdk.org/jdk/pull/26375#discussion_r2292684206
