On Thu, 10 Apr 2025 15:50:56 GMT, Ioi Lam <ik...@openjdk.org> wrote:

>> I've realized that my example cannot be expressed in the current class list 
>> format since the format doesn't distinguish between <S, SysL> and <S, AppL>, 
>> only that S is not unregistered. The existing implementation will always use 
>> <S, SysL> as will the new one. It feels like a flaw to me but it is not a 
>> flaw of this patch.
>
> Currently we have some restrictions if`-Djava.system.class.loader=` is 
> specified
> 
> - we disable full module graph archiving: 
> https://github.com/openjdk/jdk/blob/0e223f1456c14efdb423595bee3444d5e26db7c6/src/hotspot/share/cds/cdsConfig.cpp#L286
> - we disable loading archived classes for platform and system loaders: 
> https://github.com/openjdk/jdk/blob/0e223f1456c14efdb423595bee3444d5e26db7c6/src/hotspot/share/cds/filemap.cpp#L1874-L1886
> 
> I think we should extend this limitation further (in a separate issue)
> 
> - At dump time, dump only boot classes (no platform, system or unregistered)
> - At run time, load only boot classes (no platform, system or unregistered)
> 
> I filed [JDK-8354315](https://bugs.openjdk.org/browse/JDK-8354315)

In the current JDK (with or without this patch), in your scenario, I think the 
name "S" will be printed twice with two different IDs. If a Child class named 
"C" is loaded by a custom loader, it will refer to one of the IDs. Depending on 
the order of loading, it might refer to the first or the second ID.

During dump time, we will try to load this class twice, but both attempts will 
result in the same class (defined by the user-defined system class loader). 
When resolving the unregistered class, using either ID will give you the 
correct super class ...

Anyway, this seems too fragile. I think we should fix 
[JDK-8354315](https://bugs.openjdk.org/browse/JDK-8354315) before integrating 
this patch. Or, maybe we can include the fixes of that bug in this PR as well 
(and then close that bug as a duplicate of this one). What do you think?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24223#discussion_r2037756333

Reply via email to