elharo commented on code in PR #3464:
URL: https://github.com/apache/maven-surefire/pull/3464#discussion_r3967723594
##########
surefire-api/src/main/java/org/apache/maven/surefire/api/report/StackWalkerStrategy.java:
##########
@@ -55,7 +55,13 @@ final class StackWalkerStrategy {
private static final Method FRAME_METHOD_NAME; //
StackWalker.StackFrame.getMethodName() -> String
static {
- ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+ // java.lang.StackWalker is a JDK class; do not depend on the current
thread's TCCL, which can be null
+ // (Attach Listener, user threads that cleared it). A null TCCL used
to NPE in tryLoadClass and poison
Review Comment:
only describe current code, not what uied to happen
##########
surefire-api/src/main/java/org/apache/maven/surefire/api/util/ReflectionUtils.java:
##########
@@ -148,9 +148,12 @@ public static <T> T instantiateObject(String className,
Class<?>[] types, Object
@SuppressWarnings("checkstyle:emptyblock")
public static Class<?> tryLoadClass(ClassLoader classLoader, String
className) {
+ if (classLoader == null) {
+ return null;
+ }
try {
return classLoader.loadClass(className);
- } catch (NoClassDefFoundError | ClassNotFoundException ignore) {
+ } catch (NoClassDefFoundError | ClassNotFoundException |
RuntimeException ignore) {
Review Comment:
return null should probably move into this catch block
Don't catch RuntimeException. Only catch more specific exceptions you
actually expect. Otherwise we'll hide bugs we should know about
##########
surefire-api/src/main/java/org/apache/maven/surefire/api/report/StackWalkerStrategy.java:
##########
@@ -55,7 +55,13 @@ final class StackWalkerStrategy {
private static final Method FRAME_METHOD_NAME; //
StackWalker.StackFrame.getMethodName() -> String
static {
- ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+ // java.lang.StackWalker is a JDK class; do not depend on the current
thread's TCCL, which can be null
+ // (Attach Listener, user threads that cleared it). A null TCCL used
to NPE in tryLoadClass and poison
Review Comment:
only describe current code, not what used to happen
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
