From 2a3d5c4e0e25d17976f9cdb7aa13e945e1f7b4a2 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 20 Sep 2023 10:44:43 +0100 Subject: [PATCH] Tolerate parallel init of SLF4J Closes gh-37477 --- .../logging/logback/LogbackLoggingSystem.java | 18 ++++- ...gingSystemParallelInitializationTests.java | 76 +++++++++++++++++++ src/checkstyle/checkstyle-suppressions.xml | 1 + 3 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemParallelInitializationTests.java diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java index ad46546beb..335802a28c 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java @@ -43,6 +43,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.slf4j.Marker; import org.slf4j.bridge.SLF4JBridgeHandler; +import org.slf4j.helpers.SubstituteLoggerFactory; import org.springframework.aot.AotDetector; import org.springframework.beans.factory.aot.BeanFactoryInitializationAotContribution; @@ -384,7 +385,7 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF } private LoggerContext getLoggerContext() { - ILoggerFactory factory = LoggerFactory.getILoggerFactory(); + ILoggerFactory factory = getLoggerFactory(); Assert.isInstanceOf(LoggerContext.class, factory, () -> String.format( "LoggerFactory is not a Logback LoggerContext but Logback is on " @@ -396,6 +397,21 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF return (LoggerContext) factory; } + private ILoggerFactory getLoggerFactory() { + ILoggerFactory factory = LoggerFactory.getILoggerFactory(); + while (factory instanceof SubstituteLoggerFactory) { + try { + Thread.sleep(50); + } + catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + throw new IllegalStateException("Interrupted while waiting for non-subtitute logger factory", ex); + } + factory = LoggerFactory.getILoggerFactory(); + } + return factory; + } + private Object getLocation(ILoggerFactory factory) { try { ProtectionDomain protectionDomain = factory.getClass().getProtectionDomain(); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemParallelInitializationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemParallelInitializationTests.java new file mode 100644 index 0000000000..2a1b69567b --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemParallelInitializationTests.java @@ -0,0 +1,76 @@ +/* + * Copyright 2012-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.logging.logback; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +import ch.qos.logback.classic.LoggerContext; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +import org.springframework.boot.logging.LoggingSystem; +import org.springframework.boot.testsupport.classpath.ForkedClassPath; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for parallel initialization of {@link LogbackLoggingSystem} that are separate + * from {@link LogbackLoggingSystemTests}. This isolation allows them to have complete + * control over how and when the logging system is initialized. + * + * @author Andy Wilkinson + */ +class LogbackLoggingSystemParallelInitializationTests { + + private final LoggingSystem loggingSystem = LoggingSystem + .get(LogbackLoggingSystemParallelInitializationTests.class.getClassLoader()); + + @AfterEach + void cleanUp() { + this.loggingSystem.cleanUp(); + ((LoggerContext) LoggerFactory.getILoggerFactory()).stop(); + } + + @Test + @ForkedClassPath + void noExceptionsAreThrownWhenBeforeInitializeIsCalledInParallel() { + List threads = new ArrayList<>(); + List exceptions = new CopyOnWriteArrayList<>(); + for (int i = 0; i < 10; i++) { + Thread thread = new Thread(() -> this.loggingSystem.beforeInitialize()); + thread.setUncaughtExceptionHandler((t, ex) -> exceptions.add(ex)); + threads.add(thread); + } + threads.forEach(Thread::start); + threads.forEach(this::join); + assertThat(exceptions).isEmpty(); + } + + private void join(Thread thread) { + try { + thread.join(); + } + catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + throw new RuntimeException(ex); + } + } + +} diff --git a/src/checkstyle/checkstyle-suppressions.xml b/src/checkstyle/checkstyle-suppressions.xml index fdf75ff995..6f5669c328 100644 --- a/src/checkstyle/checkstyle-suppressions.xml +++ b/src/checkstyle/checkstyle-suppressions.xml @@ -10,6 +10,7 @@ +