From 9349d64f95d0ee4e6ee0057e7376d15f1d15b37c Mon Sep 17 00:00:00 2001 From: Ceki Gulcu Date: Fri, 7 Feb 2025 10:05:06 +0100 Subject: [PATCH] In response to issue 450, moved the binding of the MDCAdapter in the MDC class from the static initializer to the LoggerFactory.bind method. In case MDC methods are called before LoggerFactory, the various methods in MDC have been modified to go through the getMDCAdapter method which checks for this rare case. https://github.com/qos-ch/slf4j/issues/450 Signed-off-by: Ceki Gulcu --- .../main/java/org/slf4j/LoggerFactory.java | 15 +++ slf4j-api/src/main/java/org/slf4j/MDC.java | 91 ++++++++++++------- .../org/slf4j/basicTests/NoBindingTest.java | 12 +++ .../org/slf4j/jul/JULServiceProvider.java | 6 +- .../nop/MultithreadedInitializationTest.java | 37 ++++---- .../reload4j/Reload4jServiceProvider.java | 6 +- .../slf4j/simple/SimpleServiceProvider.java | 7 +- 7 files changed, 110 insertions(+), 64 deletions(-) diff --git a/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java b/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java index 9ad6b8a1..c6cee5db 100755 --- a/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java +++ b/slf4j-api/src/main/java/org/slf4j/LoggerFactory.java @@ -47,6 +47,7 @@ import org.slf4j.helpers.Reporter; import org.slf4j.helpers.SubstituteLogger; import org.slf4j.helpers.SubstituteServiceProvider; import org.slf4j.helpers.Util; +import org.slf4j.spi.MDCAdapter; import org.slf4j.spi.SLF4JServiceProvider; /** @@ -195,6 +196,7 @@ public final class LoggerFactory { reportMultipleBindingAmbiguity(providersList); if (providersList != null && !providersList.isEmpty()) { PROVIDER = providersList.get(0); + earlyBindMDCAdapter(); // SLF4JServiceProvider.initialize() is intended to be called here and nowhere else. PROVIDER.initialize(); INITIALIZATION_STATE = SUCCESSFUL_INITIALIZATION; @@ -215,6 +217,19 @@ public final class LoggerFactory { } } + /** + * The value of PROVIDER.getMDCAdapter() can be null while PROVIDER has not yet initialized. + * + * However, + * + */ + private static void earlyBindMDCAdapter() { + MDCAdapter mdcAdapter = PROVIDER.getMDCAdapter(); + if(mdcAdapter != null) { + MDC.setMDCAdapter(mdcAdapter); + } + } + static SLF4JServiceProvider loadExplicitlySpecified(ClassLoader classLoader) { String explicitlySpecified = System.getProperty(PROVIDER_PROPERTY_KEY); if (null == explicitlySpecified || explicitlySpecified.isEmpty()) { diff --git a/slf4j-api/src/main/java/org/slf4j/MDC.java b/slf4j-api/src/main/java/org/slf4j/MDC.java index 6b0feded..8625af3b 100644 --- a/slf4j-api/src/main/java/org/slf4j/MDC.java +++ b/slf4j-api/src/main/java/org/slf4j/MDC.java @@ -28,10 +28,7 @@ import java.io.Closeable; import java.util.Deque; import java.util.Map; -import org.slf4j.helpers.BasicMDCAdapter; -import org.slf4j.helpers.NOPMDCAdapter; -import org.slf4j.helpers.Reporter; -import org.slf4j.helpers.Util; +import org.slf4j.helpers.*; import org.slf4j.spi.MDCAdapter; import org.slf4j.spi.SLF4JServiceProvider; @@ -68,7 +65,7 @@ public class MDC { static final String NULL_MDCA_URL = "http://www.slf4j.org/codes.html#null_MDCA"; private static final String MDC_APAPTER_CANNOT_BE_NULL_MESSAGE = "MDCAdapter cannot be null. See also " + NULL_MDCA_URL; static final String NO_STATIC_MDC_BINDER_URL = "http://www.slf4j.org/codes.html#no_static_mdc_binder"; - static MDCAdapter mdcAdapter; + static MDCAdapter MDC_ADAPTER; /** * An adapter to remove the key when done. @@ -88,17 +85,30 @@ public class MDC { private MDC() { } - static { + private static MDCAdapter getMDCAdapterGivenByProvider() { SLF4JServiceProvider provider = LoggerFactory.getProvider(); - if (provider != null) { + if(provider != null) { + // If you wish to change the mdc adapter, setting the MDC.MDCAdapter variable might be insufficient. + // Keep in mind that the provider *might* perform additional internal mdcAdapter assignments that + // you would also need to replicate/adapt. + // obtain and attach the MDCAdapter from the provider - // If you wish to change the adapter, Setting the MDC.mdcAdapter variable might not be enough as - // the provider might perform additional assignments that you would need to replicate/adapt. - mdcAdapter = provider.getMDCAdapter(); + + final MDCAdapter anAdapter = provider.getMDCAdapter(); + emitTemporaryMDCAdapterWarningIfNeeded(provider); + return anAdapter; } else { Reporter.error("Failed to find provider."); Reporter.error("Defaulting to no-operation MDCAdapter implementation."); - mdcAdapter = new NOPMDCAdapter(); + return new NOPMDCAdapter(); + } + } + + private static void emitTemporaryMDCAdapterWarningIfNeeded(SLF4JServiceProvider provider) { + boolean isSubstitute = provider instanceof SubstituteServiceProvider; + if(isSubstitute) { + Reporter.info("Temporary mdcAdapter given by SubstituteServiceProvider."); + Reporter.info("This mdcAdapter will be replaced after backend initialization has completed."); } } @@ -121,10 +131,10 @@ public class MDC { if (key == null) { throw new IllegalArgumentException("key parameter cannot be null"); } - if (mdcAdapter == null) { + if (getMDCAdapter() == null) { throw new IllegalStateException(MDC_APAPTER_CANNOT_BE_NULL_MESSAGE); } - mdcAdapter.put(key, val); + getMDCAdapter().put(key, val); } /** @@ -177,10 +187,10 @@ public class MDC { throw new IllegalArgumentException("key parameter cannot be null"); } - if (mdcAdapter == null) { + if (getMDCAdapter() == null) { throw new IllegalStateException(MDC_APAPTER_CANNOT_BE_NULL_MESSAGE); } - return mdcAdapter.get(key); + return getMDCAdapter().get(key); } /** @@ -198,20 +208,20 @@ public class MDC { throw new IllegalArgumentException("key parameter cannot be null"); } - if (mdcAdapter == null) { + if (getMDCAdapter() == null) { throw new IllegalStateException(MDC_APAPTER_CANNOT_BE_NULL_MESSAGE); } - mdcAdapter.remove(key); + getMDCAdapter().remove(key); } /** * Clear all entries in the MDC of the underlying implementation. */ public static void clear() { - if (mdcAdapter == null) { + if (getMDCAdapter() == null) { throw new IllegalStateException(MDC_APAPTER_CANNOT_BE_NULL_MESSAGE); } - mdcAdapter.clear(); + getMDCAdapter().clear(); } /** @@ -222,10 +232,10 @@ public class MDC { * @since 1.5.1 */ public static Map getCopyOfContextMap() { - if (mdcAdapter == null) { + if (getMDCAdapter() == null) { throw new IllegalStateException(MDC_APAPTER_CANNOT_BE_NULL_MESSAGE); } - return mdcAdapter.getCopyOfContextMap(); + return getMDCAdapter().getCopyOfContextMap(); } /** @@ -240,23 +250,40 @@ public class MDC { * @since 1.5.1 */ public static void setContextMap(Map contextMap) { - if (mdcAdapter == null) { + if (getMDCAdapter() == null) { throw new IllegalStateException(MDC_APAPTER_CANNOT_BE_NULL_MESSAGE); } - mdcAdapter.setContextMap(contextMap); + getMDCAdapter().setContextMap(contextMap); } /** * Returns the MDCAdapter instance currently in use. - * + * + * Since 2.0.17, if the MDCAdapter instance is null, then this method set it to use + * the adapter returned by the SLF4JProvider. However, in the vast majority of cases + * the MDCAdapter will be set earlier (during initialization) by {@link LoggerFactory}. + * * @return the MDcAdapter instance currently in use. * @since 1.4.2 */ public static MDCAdapter getMDCAdapter() { - return mdcAdapter; + if(MDC_ADAPTER == null) { + MDC_ADAPTER = getMDCAdapterGivenByProvider(); + } + return MDC_ADAPTER; } - + /** + * Set MDCAdapter instance to use. + * + * @since 2.0.17 + */ + static void setMDCAdapter(MDCAdapter anMDCAdapter) { + if(anMDCAdapter == null) { + throw new IllegalStateException(MDC_APAPTER_CANNOT_BE_NULL_MESSAGE); + } + MDC_ADAPTER = anMDCAdapter; + } /** * Push a value into the deque(stack) referenced by 'key'. @@ -266,10 +293,10 @@ public class MDC { * @since 2.0.0 */ static public void pushByKey(String key, String value) { - if (mdcAdapter == null) { + if (getMDCAdapter() == null) { throw new IllegalStateException(MDC_APAPTER_CANNOT_BE_NULL_MESSAGE); } - mdcAdapter.pushByKey(key, value); + getMDCAdapter().pushByKey(key, value); } /** @@ -280,10 +307,10 @@ public class MDC { * @since 2.0.0 */ static public String popByKey(String key) { - if (mdcAdapter == null) { + if (getMDCAdapter() == null) { throw new IllegalStateException(MDC_APAPTER_CANNOT_BE_NULL_MESSAGE); } - return mdcAdapter.popByKey(key); + return getMDCAdapter().popByKey(key); } /** @@ -295,9 +322,9 @@ public class MDC { * @since 2.0.0 */ public Deque getCopyOfDequeByKey(String key) { - if (mdcAdapter == null) { + if (getMDCAdapter() == null) { throw new IllegalStateException(MDC_APAPTER_CANNOT_BE_NULL_MESSAGE); } - return mdcAdapter.getCopyOfDequeByKey(key); + return getMDCAdapter().getCopyOfDequeByKey(key); } } diff --git a/slf4j-api/src/test/java/org/slf4j/basicTests/NoBindingTest.java b/slf4j-api/src/test/java/org/slf4j/basicTests/NoBindingTest.java index 8e1dd16c..9a1a54e5 100644 --- a/slf4j-api/src/test/java/org/slf4j/basicTests/NoBindingTest.java +++ b/slf4j-api/src/test/java/org/slf4j/basicTests/NoBindingTest.java @@ -29,6 +29,8 @@ import static org.junit.Assert.assertTrue; import java.util.Random; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,11 +39,21 @@ import org.slf4j.Marker; import org.slf4j.MarkerFactory; import org.slf4j.helpers.BasicMarker; import org.slf4j.helpers.NOPLogger; +import org.slf4j.helpers.Reporter; public class NoBindingTest { int diff = new Random().nextInt(10000); + @Before + public void setUp() throws Exception { + System.setProperty(Reporter.SLF4J_INTERNAL_VERBOSITY_KEY, "debug"); + } + @After + public void tearDown() throws Exception { + System.clearProperty(Reporter.SLF4J_INTERNAL_VERBOSITY_KEY); + } + @Test public void testLogger() { Logger logger = LoggerFactory.getLogger(NoBindingTest.class); diff --git a/slf4j-jdk14/src/main/java/org/slf4j/jul/JULServiceProvider.java b/slf4j-jdk14/src/main/java/org/slf4j/jul/JULServiceProvider.java index 5e9a74ed..d3d48519 100755 --- a/slf4j-jdk14/src/main/java/org/slf4j/jul/JULServiceProvider.java +++ b/slf4j-jdk14/src/main/java/org/slf4j/jul/JULServiceProvider.java @@ -17,8 +17,8 @@ public class JULServiceProvider implements SLF4JServiceProvider { public static String REQUESTED_API_VERSION = "2.0.99"; // !final private ILoggerFactory loggerFactory; - private IMarkerFactory markerFactory; - private MDCAdapter mdcAdapter; + private IMarkerFactory markerFactory = new BasicMarkerFactory(); + private MDCAdapter mdcAdapter = new BasicMDCAdapter(); @Override public ILoggerFactory getLoggerFactory() { @@ -42,7 +42,5 @@ public class JULServiceProvider implements SLF4JServiceProvider { @Override public void initialize() { loggerFactory = new JDK14LoggerFactory(); - markerFactory = new BasicMarkerFactory(); - mdcAdapter = new BasicMDCAdapter(); } } diff --git a/slf4j-nop/src/test/java/org/slf4j/nop/MultithreadedInitializationTest.java b/slf4j-nop/src/test/java/org/slf4j/nop/MultithreadedInitializationTest.java index dabef97c..793f3d10 100755 --- a/slf4j-nop/src/test/java/org/slf4j/nop/MultithreadedInitializationTest.java +++ b/slf4j-nop/src/test/java/org/slf4j/nop/MultithreadedInitializationTest.java @@ -1,7 +1,7 @@ /** * Copyright (c) 2004-2016 QOS.ch * All rights reserved. - * + *

* Permission is hereby granted, free of charge, to any person obtaining * a copy of this software and associated documentation files (the * "Software"), to deal in the Software without restriction, including @@ -9,10 +9,10 @@ * distribute, sublicense, and/or sell copies of the Software, and to * permit persons to whom the Software is furnished to do so, subject to * the following conditions: - * + *

* The above copyright notice and this permission notice shall be * included in all copies or substantial portions of the Software. - * + *

* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND @@ -20,21 +20,9 @@ * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - * */ package org.slf4j.nop; -import static org.junit.Assert.assertEquals; -import static org.slf4j.helpers.Reporter.SLF4J_INTERNAL_VERBOSITY_KEY; - -import java.io.PrintStream; -import java.util.ArrayList; -import java.util.List; -import java.util.Random; -import java.util.concurrent.BrokenBarrierException; -import java.util.concurrent.CyclicBarrier; -import java.util.concurrent.atomic.AtomicLong; - import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -43,6 +31,15 @@ import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactoryFriend; import org.slf4j.helpers.StringPrintStream; +import java.io.PrintStream; +import java.util.Random; +import java.util.concurrent.BrokenBarrierException; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.atomic.AtomicLong; + +import static org.junit.Assert.assertEquals; +import static org.slf4j.helpers.Reporter.SLF4J_INTERNAL_VERBOSITY_KEY; + public class MultithreadedInitializationTest { static int NUM_LINES_IN_SLF4J_CONNECTED_WITH_PROVIDER_INFO = 1; @@ -76,7 +73,7 @@ public class MultithreadedInitializationTest { System.out.println("THREAD_COUNT=" + THREAD_COUNT); LoggerAccessingThread[] accessors = harness(); - for (LoggerAccessingThread accessor : accessors) { + for(LoggerAccessingThread accessor : accessors) { EVENT_COUNT.getAndIncrement(); accessor.logger.info("post harness"); } @@ -91,13 +88,13 @@ public class MultithreadedInitializationTest { private static LoggerAccessingThread[] harness() throws InterruptedException, BrokenBarrierException { LoggerAccessingThread[] threads = new LoggerAccessingThread[THREAD_COUNT]; final CyclicBarrier barrier = new CyclicBarrier(THREAD_COUNT + 1); - for (int i = 0; i < THREAD_COUNT; i++) { + for(int i = 0; i < THREAD_COUNT; i++) { threads[i] = new LoggerAccessingThread(barrier, i); threads[i].start(); } barrier.await(); - for (int i = 0; i < THREAD_COUNT; i++) { + for(int i = 0; i < THREAD_COUNT; i++) { threads[i].join(); } return threads; @@ -123,7 +120,9 @@ public class MultithreadedInitializationTest { logger.info("in run method"); EVENT_COUNT.getAndIncrement(); } - }; + } + + ; // public static class StringPrintStream extends PrintStream { // diff --git a/slf4j-reload4j/src/main/java/org/slf4j/reload4j/Reload4jServiceProvider.java b/slf4j-reload4j/src/main/java/org/slf4j/reload4j/Reload4jServiceProvider.java index ed023b31..170cbd92 100644 --- a/slf4j-reload4j/src/main/java/org/slf4j/reload4j/Reload4jServiceProvider.java +++ b/slf4j-reload4j/src/main/java/org/slf4j/reload4j/Reload4jServiceProvider.java @@ -19,8 +19,8 @@ public class Reload4jServiceProvider implements SLF4JServiceProvider { public static String REQUESTED_API_VERSION = "2.0.99"; // !final private ILoggerFactory loggerFactory; - private IMarkerFactory markerFactory; - private MDCAdapter mdcAdapter; + private IMarkerFactory markerFactory = new BasicMarkerFactory(); + private MDCAdapter mdcAdapter = new Reload4jMDCAdapter(); public Reload4jServiceProvider() { try { @@ -34,8 +34,6 @@ public class Reload4jServiceProvider implements SLF4JServiceProvider { @Override public void initialize() { loggerFactory = new Reload4jLoggerFactory(); - markerFactory = new BasicMarkerFactory(); - mdcAdapter = new Reload4jMDCAdapter(); } @Override diff --git a/slf4j-simple/src/main/java/org/slf4j/simple/SimpleServiceProvider.java b/slf4j-simple/src/main/java/org/slf4j/simple/SimpleServiceProvider.java index 795200dc..f2462737 100755 --- a/slf4j-simple/src/main/java/org/slf4j/simple/SimpleServiceProvider.java +++ b/slf4j-simple/src/main/java/org/slf4j/simple/SimpleServiceProvider.java @@ -17,8 +17,8 @@ public class SimpleServiceProvider implements SLF4JServiceProvider { public static String REQUESTED_API_VERSION = "2.0.99"; // !final private ILoggerFactory loggerFactory; - private IMarkerFactory markerFactory; - private MDCAdapter mdcAdapter; + private IMarkerFactory = new BasicMarkerFactory(); + private MDCAdapter mdcAdapter = new NOPMDCAdapter(); public ILoggerFactory getLoggerFactory() { return loggerFactory; @@ -41,10 +41,7 @@ public class SimpleServiceProvider implements SLF4JServiceProvider { @Override public void initialize() { - loggerFactory = new SimpleLoggerFactory(); - markerFactory = new BasicMarkerFactory(); - mdcAdapter = new NOPMDCAdapter(); } }