Skip to content

Commit

Permalink
In response to issue 450, moved the binding of the MDCAdapter in the
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@ceki
ceki committed Feb 7, 2025
1 parent 0def25e commit 9349d64
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 64 deletions.
15 changes: 15 additions & 0 deletions slf4j-api/src/main/java/org/slf4j/LoggerFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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;
Expand All @@ -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()) {
Expand Down
91 changes: 59 additions & 32 deletions slf4j-api/src/main/java/org/slf4j/MDC.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand All @@ -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.");
}
}

Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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();
}

/**
Expand All @@ -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();
}

/**
Expand All @@ -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'.
Expand All @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}
}
12 changes: 12 additions & 0 deletions slf4j-api/src/test/java/org/slf4j/basicTests/NoBindingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
6 changes: 2 additions & 4 deletions slf4j-jdk14/src/main/java/org/slf4j/jul/JULServiceProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -42,7 +42,5 @@ public String getRequestedApiVersion() {
@Override
public void initialize() {
loggerFactory = new JDK14LoggerFactory();
markerFactory = new BasicMarkerFactory();
mdcAdapter = new BasicMDCAdapter();
}
}
37 changes: 18 additions & 19 deletions slf4j-nop/src/test/java/org/slf4j/nop/MultithreadedInitializationTest.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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");
}
Expand All @@ -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;
Expand All @@ -123,7 +120,9 @@ public void run() {
logger.info("in run method");
EVENT_COUNT.getAndIncrement();
}
};
}

;

// public static class StringPrintStream extends PrintStream {
//
Expand Down
Loading

0 comments on commit 9349d64

Please sign in to comment.