You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by ck...@apache.org on 2021/03/24 01:59:07 UTC
[logging-log4j2] branch release-2.x updated: LOG4J2-2940: Context
selectors are aware of ClassLoader dependency
This is an automated email from the ASF dual-hosted git repository.
ckozak pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/release-2.x by this push:
new c217cd5 LOG4J2-2940: Context selectors are aware of ClassLoader dependency
c217cd5 is described below
commit c217cd5b06a63e1948fdea8be8e79e6a1deaf859
Author: Carter Kozak <ck...@apache.org>
AuthorDate: Tue Mar 16 15:21:21 2021 -0400
LOG4J2-2940: Context selectors are aware of ClassLoader dependency
This allows LoggerContext lookups to avoid searching for the calling
class to discover a classloader when the ContextSelector implementation
is declared to be agnostic to the class loader.
Custom LoggerContextFactory and ContextSelector implementations
should be updated to override the new `isClassLoaderDependent` method.
---
.../logging/log4j/simple/SimpleLoggerContextFactory.java | 5 +++++
.../org/apache/logging/log4j/spi/LoggerContextFactory.java | 13 +++++++++++++
.../org/apache/logging/log4j/TestLoggerContextFactory.java | 5 +++++
.../apache/logging/log4j/core/impl/Log4jContextFactory.java | 5 +++++
.../logging/log4j/core/selector/BasicContextSelector.java | 5 +++++
.../log4j/core/selector/ClassLoaderContextSelector.java | 6 ++++++
.../apache/logging/log4j/core/selector/ContextSelector.java | 12 +++++++++++-
.../logging/log4j/core/selector/JndiContextSelector.java | 5 +++++
.../log4j/core/async/AsyncLoggerContextSelectorTest.java | 5 +++++
.../core/async/AsyncLoggerCustomSelectorLocationTest.java | 5 +++++
.../apache/logging/log4j/core/async/AsyncLoggerTest.java | 2 ++
.../log4j/core/selector/BasicContextSelectorTest.java | 9 +++++++--
.../main/java/org/apache/logging/log4j/jcl/LogAdapter.java | 5 ++++-
.../apache/logging/log4j/jpl/Log4jSystemLoggerAdapter.java | 5 ++++-
.../org/apache/logging/log4j/jul/AbstractLoggerAdapter.java | 5 ++++-
.../java/org/apache/logging/slf4j/Log4jLoggerFactory.java | 8 ++++++--
.../java/org/apache/logging/slf4j/Log4jLoggerFactory.java | 8 ++++++--
.../org/apache/logging/slf4j/SLF4JLoggerContextFactory.java | 8 +++++++-
src/changes/changes.xml | 5 +++++
19 files changed, 110 insertions(+), 11 deletions(-)
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContextFactory.java b/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContextFactory.java
index 8ea57a8..b78a3de 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContextFactory.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContextFactory.java
@@ -44,4 +44,9 @@ public class SimpleLoggerContextFactory implements LoggerContextFactory {
public void removeContext(final LoggerContext removeContext) {
// do nothing
}
+
+ @Override
+ public boolean isClassLoaderDependent() {
+ return false;
+ }
}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextFactory.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextFactory.java
index 7855098..4c1caf3 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextFactory.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerContextFactory.java
@@ -87,4 +87,17 @@ public interface LoggerContextFactory {
* @param context The context to remove.
*/
void removeContext(LoggerContext context);
+
+ /**
+ * Determines whether or not this factory and perhaps the underlying
+ * ContextSelector behavior depend on the callers classloader.
+ *
+ * This method should be overridden by implementations, however a default method is provided which always
+ * returns {@code true} to preserve the old behavior.
+ *
+ * @since 2.15.0
+ */
+ default boolean isClassLoaderDependent() {
+ return true;
+ }
}
diff --git a/log4j-api/src/test/java/org/apache/logging/log4j/TestLoggerContextFactory.java b/log4j-api/src/test/java/org/apache/logging/log4j/TestLoggerContextFactory.java
index 847fedd..04ad4a3 100644
--- a/log4j-api/src/test/java/org/apache/logging/log4j/TestLoggerContextFactory.java
+++ b/log4j-api/src/test/java/org/apache/logging/log4j/TestLoggerContextFactory.java
@@ -43,4 +43,9 @@ public class TestLoggerContextFactory implements LoggerContextFactory {
@Override
public void removeContext(final LoggerContext context) {
}
+
+ @Override
+ public boolean isClassLoaderDependent() {
+ return false;
+ }
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jContextFactory.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jContextFactory.java
index 3bee68b..8ac35b4 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jContextFactory.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jContextFactory.java
@@ -374,6 +374,11 @@ public class Log4jContextFactory implements LoggerContextFactory, ShutdownCallba
}
@Override
+ public boolean isClassLoaderDependent() {
+ return selector.isClassLoaderDependent();
+ }
+
+ @Override
public Cancellable addShutdownCallback(final Runnable callback) {
return isShutdownHookEnabled() ? shutdownCallbackRegistry.addShutdownCallback(callback) : null;
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/BasicContextSelector.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/BasicContextSelector.java
index 6aa4dcd..1c19e36 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/BasicContextSelector.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/BasicContextSelector.java
@@ -71,6 +71,11 @@ public class BasicContextSelector implements ContextSelector {
}
@Override
+ public boolean isClassLoaderDependent() {
+ return false;
+ }
+
+ @Override
public List<LoggerContext> getLoggerContexts() {
final List<LoggerContext> list = new ArrayList<>();
list.add(CONTEXT);
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java
index f7e934f..2b5b0ff 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ClassLoaderContextSelector.java
@@ -158,6 +158,12 @@ public class ClassLoaderContextSelector implements ContextSelector, LoggerContex
}
@Override
+ public boolean isClassLoaderDependent() {
+ // By definition the ClassLoaderContextSelector depends on the callers class loader.
+ return true;
+ }
+
+ @Override
public List<LoggerContext> getLoggerContexts() {
final List<LoggerContext> list = new ArrayList<>();
final Collection<AtomicReference<WeakReference<LoggerContext>>> coll = CONTEXT_MAP.values();
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ContextSelector.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ContextSelector.java
index 727ea3e..31d9dce 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ContextSelector.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/ContextSelector.java
@@ -126,5 +126,15 @@ public interface ContextSelector {
*/
void removeContext(LoggerContext context);
-
+ /**
+ * Determines whether or not this ContextSelector depends on the callers classloader.
+ * This method should be overridden by implementations, however a default method is provided which always
+ * returns {@code true} to preserve the old behavior.
+ *
+ * @return true if the class loader is required.
+ * @since 2.15.0
+ */
+ default boolean isClassLoaderDependent() {
+ return true;
+ }
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/JndiContextSelector.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/JndiContextSelector.java
index 09bd0a6..181285f 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/JndiContextSelector.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/JndiContextSelector.java
@@ -180,6 +180,11 @@ public class JndiContextSelector implements NamedContextSelector {
}
@Override
+ public boolean isClassLoaderDependent() {
+ return false;
+ }
+
+ @Override
public LoggerContext removeContext(final String name) {
return CONTEXT_MAP.remove(name);
}
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/async/AsyncLoggerContextSelectorTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/async/AsyncLoggerContextSelectorTest.java
index 29f21a5..19683de 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/async/AsyncLoggerContextSelectorTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/async/AsyncLoggerContextSelectorTest.java
@@ -65,4 +65,9 @@ public class AsyncLoggerContextSelectorTest {
assertEquals(expectedContextName, context.getName());
}
+ @Test
+ public void testDependentOnClassLoader() {
+ final AsyncLoggerContextSelector selector = new AsyncLoggerContextSelector();
+ assertTrue(selector.isClassLoaderDependent());
+ }
}
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/async/AsyncLoggerCustomSelectorLocationTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/async/AsyncLoggerCustomSelectorLocationTest.java
index e5984aa..5fa8da8 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/async/AsyncLoggerCustomSelectorLocationTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/async/AsyncLoggerCustomSelectorLocationTest.java
@@ -106,5 +106,10 @@ public class AsyncLoggerCustomSelectorLocationTest {
public void removeContext(final LoggerContext context) {
// does not remove anything
}
+
+ @Override
+ public boolean isClassLoaderDependent() {
+ return false;
+ }
}
}
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/async/AsyncLoggerTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/async/AsyncLoggerTest.java
index 27c6678..fd05419 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/async/AsyncLoggerTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/async/AsyncLoggerTest.java
@@ -72,6 +72,8 @@ public class AsyncLoggerTest {
final String location = "testAsyncLogWritesToLog";
assertTrue("no location", !line1.contains(location));
+
+ assertTrue(LogManager.getFactory().isClassLoaderDependent());
}
// NOTE: only define one @Test method per test class with Async Loggers to prevent spurious failures
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/selector/BasicContextSelectorTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/selector/BasicContextSelectorTest.java
index 6e35b72..ece4082 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/selector/BasicContextSelectorTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/selector/BasicContextSelectorTest.java
@@ -25,10 +25,10 @@ import org.junit.BeforeClass;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
public final class BasicContextSelectorTest {
-
@BeforeClass
public static void beforeClass() {
System.setProperty(Constants.LOG4J_CONTEXT_SELECTOR,
@@ -47,4 +47,9 @@ public final class BasicContextSelectorTest {
LogManager.shutdown();
assertEquals(LifeCycle.State.STOPPED, context.getState());
}
-}
\ No newline at end of file
+
+ @Test
+ public void testNotDependentOnClassLoader() {
+ assertFalse(LogManager.getFactory().isClassLoaderDependent());
+ }
+}
diff --git a/log4j-jcl/src/main/java/org/apache/logging/log4j/jcl/LogAdapter.java b/log4j-jcl/src/main/java/org/apache/logging/log4j/jcl/LogAdapter.java
index 1b402e2..94e80f1 100644
--- a/log4j-jcl/src/main/java/org/apache/logging/log4j/jcl/LogAdapter.java
+++ b/log4j-jcl/src/main/java/org/apache/logging/log4j/jcl/LogAdapter.java
@@ -18,6 +18,7 @@ package org.apache.logging.log4j.jcl;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
+import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.spi.AbstractLoggerAdapter;
import org.apache.logging.log4j.spi.LoggerContext;
import org.apache.logging.log4j.util.StackLocatorUtil;
@@ -36,7 +37,9 @@ public class LogAdapter extends AbstractLoggerAdapter<Log> {
@Override
protected LoggerContext getContext() {
- return getContext(StackLocatorUtil.getCallerClass(LogFactory.class));
+ return getContext(LogManager.getFactory().isClassLoaderDependent()
+ ? StackLocatorUtil.getCallerClass(LogFactory.class)
+ : null);
}
}
diff --git a/log4j-jpl/src/main/java/org/apache/logging/log4j/jpl/Log4jSystemLoggerAdapter.java b/log4j-jpl/src/main/java/org/apache/logging/log4j/jpl/Log4jSystemLoggerAdapter.java
index 3db9b52..d6df19e 100644
--- a/log4j-jpl/src/main/java/org/apache/logging/log4j/jpl/Log4jSystemLoggerAdapter.java
+++ b/log4j-jpl/src/main/java/org/apache/logging/log4j/jpl/Log4jSystemLoggerAdapter.java
@@ -20,6 +20,7 @@ package org.apache.logging.log4j.jpl;
import java.lang.System.Logger;
import java.lang.System.LoggerFinder;
+import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.spi.AbstractLoggerAdapter;
import org.apache.logging.log4j.spi.LoggerContext;
import org.apache.logging.log4j.util.StackLocatorUtil;
@@ -38,6 +39,8 @@ public class Log4jSystemLoggerAdapter extends AbstractLoggerAdapter<Logger> {
@Override
protected LoggerContext getContext() {
- return getContext(StackLocatorUtil.getCallerClass(LoggerFinder.class));
+ return getContext(LogManager.getFactory().isClassLoaderDependent()
+ ? StackLocatorUtil.getCallerClass(LoggerFinder.class)
+ : null);
}
}
diff --git a/log4j-jul/src/main/java/org/apache/logging/log4j/jul/AbstractLoggerAdapter.java b/log4j-jul/src/main/java/org/apache/logging/log4j/jul/AbstractLoggerAdapter.java
index ef53304..9337642 100644
--- a/log4j-jul/src/main/java/org/apache/logging/log4j/jul/AbstractLoggerAdapter.java
+++ b/log4j-jul/src/main/java/org/apache/logging/log4j/jul/AbstractLoggerAdapter.java
@@ -18,6 +18,7 @@ package org.apache.logging.log4j.jul;
import java.util.logging.Logger;
+import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.spi.LoggerContext;
import org.apache.logging.log4j.util.StackLocatorUtil;
@@ -31,7 +32,9 @@ public abstract class AbstractLoggerAdapter extends org.apache.logging.log4j.spi
@Override
protected LoggerContext getContext() {
- return getContext(StackLocatorUtil.getCallerClass(java.util.logging.LogManager.class));
+ return getContext(LogManager.getFactory().isClassLoaderDependent()
+ ? StackLocatorUtil.getCallerClass(java.util.logging.LogManager.class)
+ : null);
}
}
diff --git a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java
index b54ed9e..d4b460b 100644
--- a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java
+++ b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java
@@ -41,8 +41,12 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements
@Override
protected LoggerContext getContext() {
- final Class<?> anchor = StackLocatorUtil.getCallerClass(FQCN, PACKAGE);
- return anchor == null ? LogManager.getContext() : getContext(StackLocatorUtil.getCallerClass(anchor));
+ final Class<?> anchor = LogManager.getFactory().isClassLoaderDependent()
+ ? StackLocatorUtil.getCallerClass(FQCN, PACKAGE)
+ : null;
+ return anchor == null
+ ? LogManager.getContext()
+ : getContext(StackLocatorUtil.getCallerClass(anchor));
}
private LoggerContext validateContext(final LoggerContext context) {
if (TO_SLF4J_CONTEXT.equals(context.getClass().getName())) {
diff --git a/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java b/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java
index d0d5ae6..eb5539f 100644
--- a/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java
+++ b/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java
@@ -47,8 +47,12 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements
@Override
protected LoggerContext getContext() {
- final Class<?> anchor = StackLocatorUtil.getCallerClass(FQCN, PACKAGE);
- return anchor == null ? LogManager.getContext() : getContext(StackLocatorUtil.getCallerClass(anchor));
+ final Class<?> anchor = LogManager.getFactory().isClassLoaderDependent()
+ ? StackLocatorUtil.getCallerClass(FQCN, PACKAGE)
+ : null;
+ return anchor == null
+ ? LogManager.getContext()
+ : getContext(StackLocatorUtil.getCallerClass(anchor));
}
Log4jMarkerFactory getMarkerFactory() {
diff --git a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContextFactory.java b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContextFactory.java
index 2f20685..7de053a 100644
--- a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContextFactory.java
+++ b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLoggerContextFactory.java
@@ -28,7 +28,7 @@ import org.apache.logging.log4j.util.LoaderUtil;
*/
public class SLF4JLoggerContextFactory implements LoggerContextFactory {
private static final StatusLogger LOGGER = StatusLogger.getLogger();
- private static LoggerContext context = new SLF4JLoggerContext();
+ private static final LoggerContext context = new SLF4JLoggerContext();
public SLF4JLoggerContextFactory() {
// LOG4J2-230, LOG4J2-204 (improve error reporting when misconfigured)
@@ -60,4 +60,10 @@ public class SLF4JLoggerContextFactory implements LoggerContextFactory {
@Override
public void removeContext(final LoggerContext ignored) {
}
+
+ @Override
+ public boolean isClassLoaderDependent() {
+ // context is always used
+ return false;
+ }
}
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 27d87a8..540819b 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -42,6 +42,11 @@
<action issue="LOG4J2-3044" dev="rgoers" type="add">
Add RepeatPatternConverter.
</action>
+ <action issue="LOG4J2-2940" dev="ckozak" type="add">
+ Context selectors are aware of their dependence upon the callers ClassLoader, allowing
+ basic context selectors to avoid the unnecessary overhead of walking the stack to
+ determine the caller's ClassLoader.
+ </action>
<action issue="LOG4J2-3041" dev="rgoers" type="update">
Allow a PatternSelector to be specified on GelfLayout.
</action>