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.

#450

Signed-off-by: Ceki Gulcu <ceki@qos.ch>

Author: ceki

slf4j-api/src/main/java/org/slf4j/LoggerFactory.java

@@ -47,6 +47,7 @@
 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 @@ private final static void bind() {
             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 @@ private final static void bind() {
         }
     }
 
+    /**
+     * 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()) {

slf4j-api/src/main/java/org/slf4j/MDC.java

@@ -28,10 +28,7 @@
 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 void close() {
     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 static void put(String key, String val) throws IllegalArgumentException {
         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 static String get(String key) throws IllegalArgumentException {
             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 static void remove(String key) throws IllegalArgumentException {
             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 static void clear() {
      * @since 1.5.1
      */
     public static Map<String, String> 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 static Map<String, String> getCopyOfContextMap() {
      * @since 1.5.1
      */
     public static void setContextMap(Map<String, String> 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 static MDCAdapter getMDCAdapter() {
      * @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 @@ static public void pushByKey(String key, String value) {
      * @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 @@ static public String popByKey(String key) {
      * @since 2.0.0
      */
     public Deque<String>  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);
     }
 }

slf4j-api/src/test/java/org/slf4j/basicTests/NoBindingTest.java

@@ -29,6 +29,8 @@
 
 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.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);

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 String getRequestedApiVersion() {
     @Override
     public void initialize() {
         loggerFactory = new JDK14LoggerFactory();
-        markerFactory = new BasicMarkerFactory();
-        mdcAdapter = new BasicMDCAdapter();
     }
 }

slf4j-nop/src/test/java/org/slf4j/nop/MultithreadedInitializationTest.java

@@ -1,40 +1,28 @@
 /**
  * Copyright (c) 2004-2016 QOS.ch
  * All rights reserved.
- *
+ * <p>
  * 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
  * without limitation  the rights to  use, copy, modify,  merge, publish,
  * 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:
- *
+ * <p>
  * The  above  copyright  notice  and  this permission  notice  shall  be
  * included in all copies or substantial portions of the Software.
- *
+ * <p>
  * 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
  * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
  * 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.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 void multiThreadedInitialization() throws InterruptedException, BrokenBar
         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 void multiThreadedInitialization() throws InterruptedException, BrokenBar
     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 void run() {
             logger.info("in run method");
             EVENT_COUNT.getAndIncrement();
         }
-    };
+    }
+
+    ;
 
 //    public static class StringPrintStream extends PrintStream {
 //