On Mon, 7 Apr 2025 07:46:52 GMT, Timofei Pushkin <tpush...@openjdk.org> wrote:

>> If a base class is package-private then its subclasses should have the same 
>> package name and defining class loader, otherwise `IllegalAccessError` is 
>> thrown when linking a subclass. Currently when dumping a static archive 
>> separate `URLClassLoader`s are used for each unregistered classes' source. 
>> Thus if two unregistered classes, a package-private base class and a sub 
>> class, from the same package reside in different sources 
>> `IllegalAccessError` will be thrown when linking the sub class. This can be 
>> unexpected because the app could have used a single class loader for both 
>> classes and thus not have seen the error — see `DifferentSourcesApp.java` 
>> from this patch for an example of such app.
>> 
>> This patch fixes the issue by using a single class loader for all 
>> unregistered classes. CDS does not allow classes with the same name making 
>> such solution possible.
>
> Timofei Pushkin has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Don't use URLClassPath

src/hotspot/share/cds/classListParser.cpp line 534:

> 532:   GrowableArray<InstanceKlass*> specified_interfaces = 
> get_specified_interfaces();
> 533: 
> 534:   const char* source_path = ClassLoader::uri_to_path(_source);

Is `ClassLoader::uri_to_path` necessary? I think `_source` is already a file 
path.

src/hotspot/share/cds/unregisteredClasses.cpp line 87:

> 85:                             CHECK_NULL);
> 86:     assert(result.get_type() == T_OBJECT, "just checking");
> 87:     obj = result.get_oop();

There's no need to put the above code in its own scope. Maybe do the following 
instead?


return InstanceKlass::cast(java_lang_Class::as_Klass(result.get_oop()));

src/java.base/share/classes/jdk/internal/misc/CDS.java line 444:

> 442:         protected Class<?> findClass(String name) throws 
> ClassNotFoundException {
> 443:             // Unregistered classes should be found in load(...), 
> registered classes should be
> 444:             // handeled by parent loaders

Hmm, I wonder how the reference to java.lang.Object is resolved in this case. 
When `CustomLoadee` is parsed, the ClassFileParser needs to resolve is super 
class, which should result into a call to 
`UnregisteredClassLoader::findClass()`.


            "java/lang/Object id: 0",
            "CustomLoadee id: 1 super: 0 source: .",

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24223#discussion_r2031561175
PR Review Comment: https://git.openjdk.org/jdk/pull/24223#discussion_r2031573550
PR Review Comment: https://git.openjdk.org/jdk/pull/24223#discussion_r2031586035

Reply via email to