This is an automated email from the ASF dual-hosted git repository. ckozak pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/master by this push: new 79c187a LOG4J2-2940: Context selectors are aware of ClassLoader dependency 79c187a is described below commit 79c187a3ef2dbc63dfc831d74d6854f11ef3f8f7 Author: Carter Kozak <cko...@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 | 6 ++++++ src/changes/changes.xml | 5 +++++ 19 files changed, 110 insertions(+), 9 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 ce8e242..1275bd7 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 3dcee76..37391f9 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 @@ -88,4 +88,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 4c3184d..1d8ff16 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 @@ -375,6 +375,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 a87e4b8..73bb18f 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 fb9eb68..14b776d 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,4 +126,16 @@ public interface ContextSelector { * @param context The context to remove. */ 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 36571fd..6822c96 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 @@ -174,6 +174,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 fc5cb63..4948290 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 56cbfc6..94a8776 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(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 afea703..aab205b 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 4ad0c5f..c5228e1 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) { 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 6bfc45b..0f0abae 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)); } 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 2b90008..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 @@ -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 8db44d8..05345c2 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -181,6 +181,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>