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:58:33 UTC

[logging-log4j2] branch master 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 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 <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 |  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>