Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v2]

[Mandy Chung]

On Tue, 15 Feb 2022 22:17:53 GMT, Tim Prinzing <d...@openjdk.java.net> wrote:

>> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is 
>> null
>
> Tim Prinzing has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Changes from feedback.
>   
>   - Copyright dates fixed
>   - IllegalCallerException thrown for no caller frame, and associated
>   javadoc changes
>   - test changed to look for IllegalCallerException thrown.

src/java.base/share/classes/java/lang/ClassLoader.java line 1612:

> 1610:      * In cases where {@code ClassLoader.registerAsParallelCapable} is 
> called from a context where
> 1611:      * there is no caller frame on the stack (e.g. when called directly
> 1612:      * from a JNI attached thread), {@code IllegalCallerException} is 
> thrown.

Suggestion:

     * In cases where this method is called from a context where the caller is 
not a subclass
     * {@code ClassLoader} or there is no caller frame on the stack (e.g. when 
called directly
     * from a JNI attached thread), {@code IllegalCallerException} is thrown.


Should mention the non-class loader caller as well.

src/java.base/share/classes/java/lang/ClassLoader.java line 1617:

> 1615:      * @return  {@code true} if the caller is successfully registered as
> 1616:      *          parallel capable and {@code false} if otherwise.
> 1617:      * @throws IllegalCallerException if there is no caller frame on 
> the stack.

Suggestion:

     * @throws IllegalCallerException if the caller is not a subclass of {@code 
ClassLoader}

src/java.base/share/classes/java/lang/ClassLoader.java line 1626:

> 1624:     protected static boolean registerAsParallelCapable() {
> 1625:         final Class<?> caller = Reflection.getCallerClass();
> 1626:         if (caller == null) {

Suggestion:

        if (caller == null || !ClassLoader.class.isAssignableFrom(caller)) {
             throw new IllegalCallerException(caller + " not a subclass of 
ClassLoader");
        }     


What we suggested is to throw IllegalCallerException if the caller is not a 
class loader and that will include null caller case.

test/jdk/java/lang/ClassLoader/exeNullCallerClassLoaderTest/NullCallerClassLoaderTest.java
 line 30:

> 28:  * @summary Test uses custom launcher that starts VM using JNI that 
> verifies
> 29:  *          ClassLoader.registerAsParallelCapable with null caller class 
> does not throw a NullPointerException,
> 30:  *          and instead returns false.

`@summary` needs update to reflect the new change.

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

PR: https://git.openjdk.java.net/jdk/pull/7448