You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2019/12/31 20:11:48 UTC
[GitHub] [logging-log4j2] carterkozak opened a new pull request #328:
LOG4J2-2754: LoaderUtil.getClassLoaders may discover additional loaders
carterkozak opened a new pull request #328: LOG4J2-2754: LoaderUtil.getClassLoaders may discover additional loaders
URL: https://github.com/apache/logging-log4j2/pull/328
The utility no longer erroneously returns a result with a null element in some
environments.
Also updated loadClass to avoid internally throwing and catching a null
pointer exception when getThreadContextClassLoader returns null.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [logging-log4j2] carterkozak closed pull request #328: LOG4J2-2754:
LoaderUtil.getClassLoaders may discover additional loaders
Posted by GitBox <gi...@apache.org>.
carterkozak closed pull request #328: LOG4J2-2754: LoaderUtil.getClassLoaders may discover additional loaders
URL: https://github.com/apache/logging-log4j2/pull/328
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [logging-log4j2] carterkozak commented on issue #328: LOG4J2-2754:
LoaderUtil.getClassLoaders may discover additional loaders
Posted by GitBox <gi...@apache.org>.
carterkozak commented on issue #328: LOG4J2-2754: LoaderUtil.getClassLoaders may discover additional loaders
URL: https://github.com/apache/logging-log4j2/pull/328#issuecomment-570004977
Thanks Ralph, tests all pass, I've merged this commit. It's not a clean cherry-pick to master because fbde9cd0ef was not applied to release-2.x.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request
#328: LOG4J2-2754: LoaderUtil.getClassLoaders may discover additional loaders
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #328: LOG4J2-2754: LoaderUtil.getClassLoaders may discover additional loaders
URL: https://github.com/apache/logging-log4j2/pull/328#discussion_r362274678
##########
File path: log4j-api/src/main/java/org/apache/logging/log4j/util/LoaderUtil.java
##########
@@ -106,30 +104,31 @@ public ClassLoader run() {
}
public static ClassLoader[] getClassLoaders() {
- final List<ClassLoader> classLoaders = new ArrayList<>();
+ final Collection<ClassLoader> classLoaders = new LinkedHashSet<>();
Review comment:
LinkedHashSet is more expensive in environments with few values, but performs better in the worst case without sacrificing order.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [logging-log4j2] carterkozak commented on issue #328: LOG4J2-2754:
LoaderUtil.getClassLoaders may discover additional loaders
Posted by GitBox <gi...@apache.org>.
carterkozak commented on issue #328: LOG4J2-2754: LoaderUtil.getClassLoaders may discover additional loaders
URL: https://github.com/apache/logging-log4j2/pull/328#issuecomment-569984536
@rgoers I'm not as familiar with the consumers of `LoaderUtil.getClassLoaders()` as you are, I'd appreciate your feedback before this merges, at your convenience -- I'm not in a rush to merge this, I happened to notice the loop while looking at something else.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request
#328: LOG4J2-2754: LoaderUtil.getClassLoaders may discover additional loaders
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #328: LOG4J2-2754: LoaderUtil.getClassLoaders may discover additional loaders
URL: https://github.com/apache/logging-log4j2/pull/328#discussion_r362274587
##########
File path: log4j-api/src/main/java/org/apache/logging/log4j/util/LoaderUtil.java
##########
@@ -106,30 +104,31 @@ public ClassLoader run() {
}
public static ClassLoader[] getClassLoaders() {
- final List<ClassLoader> classLoaders = new ArrayList<>();
+ final Collection<ClassLoader> classLoaders = new LinkedHashSet<>();
final ClassLoader tcl = getThreadContextClassLoader();
- classLoaders.add(tcl);
- // Some implementations may use null to represent the bootstrap class loader.
- final ClassLoader current = LoaderUtil.class.getClassLoader();
- if (current != null && current != tcl) {
- classLoaders.add(current);
- final ClassLoader parent = current.getParent();
- while (parent != null && !classLoaders.contains(parent)) {
- classLoaders.add(parent);
Review comment:
This loop failed to modify the condition
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services