From a76e84addc6ec3470f5edf3b87025dfbd49f8556 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 30 Sep 2015 09:34:32 +0100 Subject: [PATCH] =?UTF-8?q?Restore=20LoggingSystems=E2=80=99=20previous=20?= =?UTF-8?q?cleanup=20behaviour?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit reverts the changes made for gh-4026. Those changes updated each LoggingSystem to close/stop the underlying logging system as part of the clean up processing. Unfortunately, this approach doesn’t work in an environment where their are multiple application contexts and some have a shorter lifecycle than the “main” application context. In such an environment, closing an application context with a shorter lifecycle prior to the main application context being closed will close/stop the main application context’s logging system as, rather than being scoped to an application context, a logging system is shared across multiple application contexts. (The exact details of how widely shared the logging system is varies between logging systems and, in the case of Logback and Log4J2, also depends on which ContextSelector implementation is being used. --- .../boot/logging/java/JavaLoggingSystem.java | 6 ---- .../logging/log4j/Log4JLoggingSystem.java | 6 ---- .../logging/log4j2/Log4J2LoggingSystem.java | 6 ---- .../logging/logback/LogbackLoggingSystem.java | 3 -- .../logging/java/JavaLoggingSystemTests.java | 33 ------------------- .../log4j/Log4JLoggingSystemTests.java | 11 ------- .../log4j2/Log4J2LoggingSystemTests.java | 15 ++------- .../logback/LogbackLoggingSystemTests.java | 15 --------- 8 files changed, 2 insertions(+), 93 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/java/JavaLoggingSystem.java b/spring-boot/src/main/java/org/springframework/boot/logging/java/JavaLoggingSystem.java index 395b332978..fe172f0846 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/java/JavaLoggingSystem.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/java/JavaLoggingSystem.java @@ -113,10 +113,4 @@ public class JavaLoggingSystem extends AbstractLoggingSystem { logger.setLevel(LEVELS.get(level)); } - @Override - public void cleanUp() { - super.cleanUp(); - LogManager.getLogManager().reset(); - } - } diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/log4j/Log4JLoggingSystem.java b/spring-boot/src/main/java/org/springframework/boot/logging/log4j/Log4JLoggingSystem.java index 380114832e..fab44a682a 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/log4j/Log4JLoggingSystem.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/log4j/Log4JLoggingSystem.java @@ -115,10 +115,4 @@ public class Log4JLoggingSystem extends Slf4JLoggingSystem { logger.setLevel(LEVELS.get(level)); } - @Override - public void cleanUp() { - super.cleanUp(); - LogManager.shutdown(); - } - } diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java b/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java index 753e780557..4bcc9c5909 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java @@ -152,12 +152,6 @@ public class Log4J2LoggingSystem extends Slf4JLoggingSystem { loadConfiguration(location, logFile); } - @Override - public void cleanUp() { - super.cleanUp(); - getLoggerContext().stop(); - } - protected void loadConfiguration(String location, LogFile logFile) { Assert.notNull(location, "Location must not be null"); if (logFile != null) { diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java b/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java index 7a14892b02..1a12ef8674 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java @@ -37,7 +37,6 @@ import org.springframework.util.Assert; import org.springframework.util.ResourceUtils; import org.springframework.util.StringUtils; -import ch.qos.logback.classic.BasicConfigurator; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.joran.JoranConfigurator; @@ -178,8 +177,6 @@ public class LogbackLoggingSystem extends Slf4JLoggingSystem { public void cleanUp() { super.cleanUp(); getLoggerContext().getStatusManager().clear(); - getLoggerContext().stop(); - BasicConfigurator.configure(getLoggerContext()); } @Override diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/java/JavaLoggingSystemTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/java/JavaLoggingSystemTests.java index 95931ac053..c2bccf3462 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/java/JavaLoggingSystemTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/java/JavaLoggingSystemTests.java @@ -20,9 +20,6 @@ import java.io.File; import java.io.FileFilter; import java.io.IOException; import java.util.Locale; -import java.util.logging.Handler; -import java.util.logging.LogManager; -import java.util.logging.LogRecord; import org.apache.commons.logging.impl.Jdk14Logger; import org.junit.After; @@ -37,7 +34,6 @@ import org.springframework.util.StringUtils; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -164,33 +160,4 @@ public class JavaLoggingSystemTests extends AbstractLoggingSystemTests { equalTo(1)); } - @Test - public void cleanUpResetsLogManager() throws Exception { - this.loggingSystem.beforeInitialize(); - this.loggingSystem.initialize(null, null, null); - this.logger.getLogger().addHandler(new NoOpHandler()); - assertThat(this.logger.getLogger().getHandlers().length, is(equalTo(1))); - LogManager.getLogManager().reset(); - assertThat(this.logger.getLogger().getHandlers().length, is(equalTo(0))); - } - - private static final class NoOpHandler extends Handler { - - @Override - public void publish(LogRecord record) { - - } - - @Override - public void flush() { - - } - - @Override - public void close() throws SecurityException { - - } - - } - } diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/log4j/Log4JLoggingSystemTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/log4j/Log4JLoggingSystemTests.java index 5cdc2efbd2..02960d5e86 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/log4j/Log4JLoggingSystemTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/log4j/Log4JLoggingSystemTests.java @@ -139,17 +139,6 @@ public class Log4JLoggingSystemTests extends AbstractLoggingSystemTests { assertFalse(bridgeHandlerInstalled()); } - @Test - public void cleanUpStopsLogManager() { - this.loggingSystem.beforeInitialize(); - this.loggingSystem.initialize(null, null, null); - assertTrue(org.apache.log4j.LogManager.getLoggerRepository().getRootLogger() - .getAllAppenders().hasMoreElements()); - this.loggingSystem.cleanUp(); - assertFalse(org.apache.log4j.LogManager.getLoggerRepository().getRootLogger() - .getAllAppenders().hasMoreElements()); - } - private boolean bridgeHandlerInstalled() { java.util.logging.Logger rootLogger = LogManager.getLogManager().getLogger(""); Handler[] handlers = rootLogger.getHandlers(); diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java index 4ee7fe6d38..4b8c11b891 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java @@ -24,7 +24,6 @@ import java.util.List; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.config.FileConfigurationMonitor; import org.hamcrest.Matcher; @@ -239,17 +238,6 @@ public class Log4J2LoggingSystemTests extends AbstractLoggingSystemTests { } } - @Test - public void cleanupStopsContext() throws Exception { - this.loggingSystem.beforeInitialize(); - this.logger.info("Hidden"); - this.loggingSystem.initialize(null, null, null); - LoggerContext context = (LoggerContext) LogManager.getContext(false); - assertFalse(context.isStopped()); - this.loggingSystem.cleanUp(); - assertTrue(context.isStopped()); - } - private static class TestLog4J2LoggingSystem extends Log4J2LoggingSystem { private List availableClasses = new ArrayList(); @@ -259,7 +247,8 @@ public class Log4J2LoggingSystemTests extends AbstractLoggingSystemTests { } public Configuration getConfiguration() { - return ((LoggerContext) LogManager.getContext(false)).getConfiguration(); + return ((org.apache.logging.log4j.core.LoggerContext) LogManager + .getContext(false)).getConfiguration(); } @Override diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java index 4e21104376..1db49aece1 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java @@ -51,7 +51,6 @@ import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -122,7 +121,6 @@ public class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { @Test public void testBasicConfigLocation() throws Exception { this.loggingSystem.beforeInitialize(); - this.loggingSystem.initialize(this.initializationContext, null, null); ILoggerFactory factory = StaticLoggerBinder.getSingleton().getLoggerFactory(); LoggerContext context = (LoggerContext) factory; Logger root = context.getLogger(org.slf4j.Logger.ROOT_LOGGER_NAME); @@ -307,19 +305,6 @@ public class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { } } - @Test - public void cleanUpStopsContext() throws Exception { - this.loggingSystem.beforeInitialize(); - this.loggingSystem.initialize(this.initializationContext, null, null); - ILoggerFactory factory = StaticLoggerBinder.getSingleton().getLoggerFactory(); - LoggerContext context = (LoggerContext) factory; - Logger root = context.getLogger(org.slf4j.Logger.ROOT_LOGGER_NAME); - assertNotNull(root.getAppender("CONSOLE")); - - this.loggingSystem.cleanUp(); - assertNull(root.getAppender("CONSOLE")); - } - private String getLineWithText(File file, String outputSearch) throws Exception { return getLineWithText(FileCopyUtils.copyToString(new FileReader(file)), outputSearch);