Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v2]
On Tue, 15 Feb 2022 23:05:30 GMT, Mandy Chung wrote:
>> 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 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.
This also needs a test case for non-class loader caller.
I looked at the existing tests testing `registerAsParallelCapable` but I can't
find a good one to add the new test case. The closest one could be
`test/jdk/java/lang/ClassLoader/IsParallelCapable.java`. I'm okay to include
a new test case in `IsParallelCapable.java` to verify `IllegalCallerException`
if called by a non-class loader.
-
PR: https://git.openjdk.java.net/jdk/pull/7448
Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v2]
On Tue, 15 Feb 2022 22:17:53 GMT, Tim Prinzing 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
Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v2]
> 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. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7448/files - new: https://git.openjdk.java.net/jdk/pull/7448/files/c6d37069..fa66af15 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7448=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7448=00-01 Stats: 33 lines in 3 files changed: 22 ins; 2 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/7448.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7448/head:pull/7448 PR: https://git.openjdk.java.net/jdk/pull/7448
