Fixed race condition occurring in case MDC class is initialized while…

… org.slf4j.LoggerFactory is initializing logback-classic's LoggerContext. Signed-off-by: Ceki Gulcu <[email protected]>
qos-ch · Feb 11, 2025 · b507297 · b507297
1 parent f5b3bc5
commit b507297
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 31 deletions.
diff --git a/logback-classic/pom.xml b/logback-classic/pom.xml
@@ -269,6 +269,7 @@
                 --add-modules jakarta.mail
                 --add-modules jakarta.servlet
                 --add-opens ch.qos.logback.core/ch.qos.logback.core.testUtil=java.naming
+                --add-opens ch.qos.logback.classic/ch.qos.logback.classic.issue.github450=ch.qos.logback.core
                 --add-opens ch.qos.logback.classic/ch.qos.logback.classic.testUtil=ch.qos.logback.core
                 --add-opens ch.qos.logback.classic/ch.qos.logback.classic.jsonTest=ALL-UNNAMED
               </argLine>
@@ -288,6 +289,7 @@
                 <exclude>org.slf4j.implTest.InitializationOutputTest.java</exclude>
                 <exclude>ch.qos.logback.classic.util.ContextInitializerTest.java</exclude>
                 <exclude>ch.qos.logback.classic.spi.InvocationTest.java</exclude>
+                <exclude>ch.qos.logback.classic.issue.github450.SLF4JIssue450Test</exclude>
               </excludes>
             </configuration>
           </execution>
@@ -300,11 +302,15 @@
             <configuration>
               <forkCount>4</forkCount>
               <reuseForks>false</reuseForks>
+              <argLine>
+                --add-opens ch.qos.logback.classic/ch.qos.logback.classic.issue.github450=ch.qos.logback.core
+              </argLine>
               <includes>
                 <include>org.slf4j.implTest.MultithreadedInitializationTest.java</include>
                 <include>org.slf4j.implTest.InitializationOutputTest.java</include>
                 <include>ch.qos.logback.classic.util.ContextInitializerTest.java</include>
                 <include>ch.qos.logback.classic.spi.InvocationTest.java</include>
+                <include>ch.qos.logback.classic.issue.github450.SLF4JIssue450Test</include>
               </includes>
             </configuration>
           </execution>

diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/spi/LogbackServiceProvider.java b/logback-classic/src/main/java/ch/qos/logback/classic/spi/LogbackServiceProvider.java
@@ -26,24 +26,23 @@ public class LogbackServiceProvider implements SLF4JServiceProvider {
     // to avoid constant folding by the compiler, this field must *not* be final
     public static String REQUESTED_API_VERSION = "2.0.99"; // !final
 
-    private LoggerContext defaultLoggerContext;
-    private IMarkerFactory markerFactory;
-    private LogbackMDCAdapter mdcAdapter;
-    // private final ContextSelectorStaticBinder contextSelectorBinder =
-    // ContextSelectorStaticBinder.getSingleton();
-//    private static Object KEY = new Object();
-//    private volatile boolean initialized = false;
+    private LoggerContext defaultLoggerContext = new LoggerContext();
+
+
+    // org.slf4j.LoggerFactory expects providers to initialize markerFactory as early as possible.
+    private IMarkerFactory markerFactory = new BasicMarkerFactory();
+
+    // org.slf4j.LoggerFactory expects providers to initialize their MDCAdapter field
+    // as early as possible, preferably at construction time.
+    private LogbackMDCAdapter mdcAdapter = new LogbackMDCAdapter();
 
     @Override
     public void initialize() {
-        defaultLoggerContext = new LoggerContext();
         defaultLoggerContext.setName(CoreConstants.DEFAULT_CONTEXT_NAME);
-        initializeLoggerContext();
-        defaultLoggerContext.start();
-        markerFactory = new BasicMarkerFactory();
-        mdcAdapter = new LogbackMDCAdapter();
         // set the MDCAdapter for the defaultLoggerContext immediately
         defaultLoggerContext.setMDCAdapter(mdcAdapter);
+        initializeLoggerContext();
+        defaultLoggerContext.start();
     }
 
     private void initializeLoggerContext() {
@@ -68,15 +67,6 @@ private void initializeLoggerContext() {
 
     public ILoggerFactory getLoggerFactory() {
         return defaultLoggerContext;
-
-//        if (!initialized) {
-//            return defaultLoggerContext;
-//        
-//
-//        if (contextSelectorBinder.getContextSelector() == null) {
-//            throw new IllegalStateException("contextSelector cannot be null. See also " + NULL_CS_URL);
-//        }
-//        return contextSelectorBinder.getContextSelector().getLoggerContext();
     }
 
     @Override

diff --git a/logback-classic/src/test/input/issue/gh_issues_450.xml b/logback-classic/src/test/input/issue/gh_issues_450.xml
@@ -0,0 +1,42 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!DOCTYPE configuration>
+<!--
+  ~ Logback: the reliable, generic, fast and flexible logging framework.
+  ~ Copyright (C) 1999-2025, QOS.ch. All rights reserved.
+  ~
+  ~ This program and the accompanying materials are dual-licensed under
+  ~ either the terms of the Eclipse Public License v1.0 as published by
+  ~ the Eclipse Foundation
+  ~
+  ~   or (per the licensee's choosing)
+  ~
+  ~ under the terms of the GNU Lesser General Public License version 2.1
+  ~ as published by the Free Software Foundation.
+  -->
+
+
+<configuration>
+    <import class="ch.qos.logback.classic.issue.github450.Issues450LoggerContextListener"/>
+    <import class="ch.qos.logback.classic.encoder.PatternLayoutEncoder"/>
+    <import class="ch.qos.logback.core.ConsoleAppender"/>
+    <import class="ch.qos.logback.core.read.ListAppender"/>
+
+    <contextListener class="Issues450LoggerContextListener"/>
+
+    <appender name="LIST" class="ListAppender"/>
+
+    <appender name="CONSOLE" class="ConsoleAppender">
+        <encoder class="PatternLayoutEncoder">
+            <pattern>%d{HH:mm:ss.SSS} [%t] **[%X]** %-5level %logger{36} - %msg%n</pattern>
+        </encoder>
+    </appender>
+
+    <logger name="app" level="debug" additivity="false">
+        <appender-ref ref="CONSOLE"/>
+    </logger>
+
+    <root level="debug">
+        <appender-ref ref="LIST"/>
+        <appender-ref ref="CONSOLE"/>
+    </root>
+</configuration>
diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/issue/github450/SLF4JIssue450Test.java b/logback-classic/src/test/java/ch/qos/logback/classic/issue/github450/SLF4JIssue450Test.java
@@ -16,20 +16,24 @@
 
 import ch.qos.logback.classic.ClassicConstants;
 import ch.qos.logback.classic.ClassicTestConstants;
-import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.classic.spi.LoggingEvent;
 import ch.qos.logback.core.CoreConstants;
 import ch.qos.logback.core.read.ListAppender;
+import org.junit.jupiter.api.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.slf4j.MDC;
 
-public class Main {
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
-    public static void main(String[] args) {
-        System.setProperty(ClassicConstants.CONFIG_FILE_PROPERTY, "logback-classic/"+ClassicTestConstants.INPUT_PREFIX + "issue/gh_issues_450.xml");
+public class SLF4JIssue450Test {
+
+
+    @Test
+    public void smoke() {
+        System.setProperty(ClassicConstants.CONFIG_FILE_PROPERTY, ClassicTestConstants.INPUT_PREFIX + "issue/gh_issues_450.xml");
         System.setProperty(CoreConstants.STATUS_LISTENER_CLASS_KEY, "stdout");
-        Logger logger = LoggerFactory.getLogger(Main.class);
+        Logger logger = LoggerFactory.getLogger(SLF4JIssue450Test.class);
         logger.info("toto");
         ch.qos.logback.classic.Logger root = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
 
@@ -38,8 +42,8 @@ public static void main(String[] args) {
         LoggingEvent le0 = (LoggingEvent) listAppender.list.get(0);
 
         String val = le0.getMDCPropertyMap().get("issues450");
-        if(val == null) {
-            throw new RuntimeException("issues450 missing property: issues450");
-        }
+        assertNotNull(val);
+        assertEquals("12", val);
+
     }
 }
diff --git a/pom.xml b/pom.xml
@@ -74,7 +74,7 @@
     <!-- slf4j.version property is used below, in
          logback-classic/pom.xml, /logback-examples/src/main/resources/setClasspath.cmd, download.html
     -->    
-    <slf4j.version>2.0.16</slf4j.version>
+    <slf4j.version>2.0.17-SNAPSHOT</slf4j.version>
     <cal10n.version>0.8.1</cal10n.version>
     <consolePlugin.version>1.1.0</consolePlugin.version>
     <jackson.version>2.15.0</jackson.version>