RFR: 8228773: URLClassLoader constructors should include API note warning that the parent should not be null [v3]

[Sean Mullan]

On Thu, 29 May 2025 14:19:08 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review of this doc-only change which proposes to add an `@apiNote` on the constructors of `URLClassLoader` and `SecureClassLoader` to explain the current implementation of these constructors? This addresses https://bugs.openjdk.org/browse/JDK-8228773?
>> 
>> As noted in that issue, this updated documentation is to help applications be aware that a `null` value which represents the bootstrap class loader when passed for `parent` class loader will mean that the constructed `URLClassLoader` may not be able to load all platform classes.
>> 
>> Specifically, only a specific set of modules are mapped (at JDK build time) to the bootstrap class loader. Some modules like `java.sql` are visible to the platform class loader but not to the bootstrap classloader. The distinction between these class loaders is explained in the API documentation of `ClassLoader` class https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/lang/ClassLoader.html#builtinLoaders.
>> 
>> Using `null` (which represents a bootstrap class loader) for `parent` when constructing the `URLClassLoader` then means that the class loader would not be able to load some of these platform classes. For example, consider:
>> 
>> 
>> import java.net.*;
>> 
>> public class Test {
>>     public static void main(String[] args) throws Exception {
>>         System.out.println("testing against Java " + System.getProperty("java.version"));
>> 
>>         final ClassLoader cl = new URLClassLoader(new URL[0], null);
>>         final Class<?> klass = cl.loadClass("java.sql.ResultSet"); // load a platform class that belongs to the java.sql module
>>         System.out.println("loaded " + klass + " using classloader: " + klass.getClassLoader());
>> 
>>     }
>> }
>> 
>> which constructs the `URLClassLoader` with the bootstrap class loader as its parent and then attempts to load a platform class `java.sql.ResultSet`. Since this class' module is mapped to the platform class loader and not the bootstrap class loader, running this code against Java 9+ will result in a `ClassNotFoundException`:
>> 
>> 
>> testing against Java 24
>> Exception in thread "main" java.lang.ClassNotFoundException: java.sql.ResultSet
>>     at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:349)
>>     at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:557)
>>     at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:490)
>>     at Test.main(Test.java:8)
>> 
>> 
>> No new tests have been introduced and existing tes...
>
> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   additional sentence to the apiNote

src/java.base/share/classes/java/net/URLClassLoader.java line 87:

> 85:      *
> 86:      * @param      urls the URLs from which to load classes and resources
> 87:      * @param      parent the parent class loader for delegation, can be null

s/null/{@code null}/

src/java.base/share/classes/java/net/URLClassLoader.java line 123:

> 121:      * obtain protocol handlers when creating new jar URLs.
> 122:      *
> 123:      * @apiNote If the {@code parent} is specified as {@code null} (for the

I think "If {@code parent}" is more correct, since you are implicitly referring to the parameter.

src/java.base/share/classes/java/net/URLClassLoader.java line 127:

> 125:      * classes are visible.
> 126:      * See {@linkplain ClassLoader##builtinLoaders Run-time Built-in Class Loaders}
> 127:      * for information on the system class loader and other built-in class loaders.

Seems more relevant to say "for information on the bootstrap class loader and other built-in class loaders." since you specifically mention bootstrap loader in the first sentence.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25517#discussion_r2114361908
PR Review Comment: https://git.openjdk.org/jdk/pull/25517#discussion_r2114370688
PR Review Comment: https://git.openjdk.org/jdk/pull/25517#discussion_r2114366997