From 45ad1557c36426878aca0631e33289786a349084 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 3 Aug 2022 11:21:02 +0100 Subject: [PATCH] Perform LoggingSystem cleanup after web server shutdown Update `LoggingApplicationListener` so that logging system cleanup is performed by a `SmartLifecycle` phased after web server shutdown. Prior to this commit, cleanup occurred on the `ContextClosedEvent` which was published before Lifecycle beans were stopped. This meant that any exceptions output during web server shutdown were not logged. Fixes gh-9457 --- .../logging/LoggingApplicationListener.java | 60 ++++++- .../LoggingApplicationListenerTests.java | 154 ++++++++++++------ 2 files changed, 160 insertions(+), 54 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java index 64ba5168c4..54a19afd0b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java @@ -45,6 +45,8 @@ import org.springframework.boot.logging.LoggingSystemProperties; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationListener; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.SmartLifecycle; import org.springframework.context.event.ContextClosedEvent; import org.springframework.context.event.GenericApplicationListener; import org.springframework.core.Ordered; @@ -136,6 +138,11 @@ public class LoggingApplicationListener implements GenericApplicationListener { */ public static final String LOGGER_GROUPS_BEAN_NAME = "springBootLoggerGroups"; + /** + * The name of the {@link Lifecycle} bean used to handle cleanup. + */ + private static final String LOGGING_LIFECYCLE_BEAN_NAME = "springBootLoggingLifecycle"; + private static final Map> DEFAULT_GROUP_LOGGERS; static { MultiValueMap loggers = new LinkedMultiValueMap<>(); @@ -218,9 +225,8 @@ public class LoggingApplicationListener implements GenericApplicationListener { else if (event instanceof ApplicationPreparedEvent) { onApplicationPreparedEvent((ApplicationPreparedEvent) event); } - else if (event instanceof ContextClosedEvent - && ((ContextClosedEvent) event).getApplicationContext().getParent() == null) { - onContextClosedEvent(); + else if (event instanceof ContextClosedEvent) { + onContextClosedEvent((ContextClosedEvent) event); } else if (event instanceof ApplicationFailedEvent) { onApplicationFailedEvent(); @@ -241,7 +247,8 @@ public class LoggingApplicationListener implements GenericApplicationListener { } private void onApplicationPreparedEvent(ApplicationPreparedEvent event) { - ConfigurableListableBeanFactory beanFactory = event.getApplicationContext().getBeanFactory(); + ConfigurableApplicationContext applicationContext = event.getApplicationContext(); + ConfigurableListableBeanFactory beanFactory = applicationContext.getBeanFactory(); if (!beanFactory.containsBean(LOGGING_SYSTEM_BEAN_NAME)) { beanFactory.registerSingleton(LOGGING_SYSTEM_BEAN_NAME, this.loggingSystem); } @@ -251,20 +258,29 @@ public class LoggingApplicationListener implements GenericApplicationListener { if (this.loggerGroups != null && !beanFactory.containsBean(LOGGER_GROUPS_BEAN_NAME)) { beanFactory.registerSingleton(LOGGER_GROUPS_BEAN_NAME, this.loggerGroups); } + if (!beanFactory.containsBean(LOGGING_LIFECYCLE_BEAN_NAME) && applicationContext.getParent() == null) { + beanFactory.registerSingleton(LOGGING_LIFECYCLE_BEAN_NAME, new Lifecycle()); + } } - private void onContextClosedEvent() { - if (this.loggingSystem != null) { - this.loggingSystem.cleanUp(); + private void onContextClosedEvent(ContextClosedEvent event) { + ApplicationContext applicationContext = event.getApplicationContext(); + if (applicationContext.getParent() != null || applicationContext.containsBean(LOGGING_LIFECYCLE_BEAN_NAME)) { + return; } + cleanupLoggingSystem(); } - private void onApplicationFailedEvent() { + void cleanupLoggingSystem() { if (this.loggingSystem != null) { this.loggingSystem.cleanUp(); } } + private void onApplicationFailedEvent() { + cleanupLoggingSystem(); + } + /** * Initialize the logging system according to preferences expressed through the * {@link Environment} and the classpath. @@ -438,4 +454,32 @@ public class LoggingApplicationListener implements GenericApplicationListener { this.parseArgs = parseArgs; } + private class Lifecycle implements SmartLifecycle { + + private volatile boolean running; + + @Override + public void start() { + this.running = true; + } + + @Override + public void stop() { + this.running = false; + cleanupLoggingSystem(); + } + + @Override + public boolean isRunning() { + return this.running; + } + + @Override + public int getPhase() { + // Shutdown late and always after WebServerStartStopLifecycle + return Integer.MIN_VALUE + 1; + } + + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java index d27ca98a43..9ac81baa25 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2021 the original author or authors. + * Copyright 2012-2022 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. @@ -46,6 +46,7 @@ import org.slf4j.impl.StaticLoggerBinder; import org.springframework.boot.DefaultBootstrapContext; import org.springframework.boot.SpringApplication; +import org.springframework.boot.WebApplicationType; import org.springframework.boot.context.event.ApplicationFailedEvent; import org.springframework.boot.context.event.ApplicationStartingEvent; import org.springframework.boot.context.properties.bind.BindException; @@ -66,6 +67,9 @@ import org.springframework.boot.testsupport.system.OutputCaptureExtension; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationListener; import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.SmartLifecycle; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; import org.springframework.context.event.ContextClosedEvent; import org.springframework.context.event.SimpleApplicationEventMulticaster; import org.springframework.context.support.GenericApplicationContext; @@ -95,7 +99,7 @@ class LoggingApplicationListenerTests { private static final String[] NO_ARGS = {}; - private final LoggingApplicationListener initializer = new LoggingApplicationListener(); + private final LoggingApplicationListener listener = new LoggingApplicationListener(); private final LoggerContext loggerContext = (LoggerContext) StaticLoggerBinder.getSingleton().getLoggerFactory(); @@ -103,7 +107,7 @@ class LoggingApplicationListenerTests { private final DefaultBootstrapContext bootstrapContext = new DefaultBootstrapContext(); - private final SpringApplication springApplication = new SpringApplication(); + private final SpringApplication springApplication = new SpringApplication(TestConfiguration.class); private final GenericApplicationContext context = new GenericApplicationContext(); @@ -146,7 +150,7 @@ class LoggingApplicationListenerTests { @Test void baseConfigLocation() { - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.info("Hello world", new RuntimeException("Expected")); assertThat(this.output).contains("Hello world"); assertThat(this.output).doesNotContain("???"); @@ -157,7 +161,7 @@ class LoggingApplicationListenerTests { @Test void overrideConfigLocation() { addPropertiesToEnvironment(this.context, "logging.config=classpath:logback-nondefault.xml"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.info("Hello world"); assertThat(this.output).contains("Hello world").doesNotContain("???").startsWith("null ").endsWith("BOOTBOOT"); } @@ -165,7 +169,7 @@ class LoggingApplicationListenerTests { @Test void trailingWhitespaceInLoggingConfigShouldBeTrimmed() { addPropertiesToEnvironment(this.context, "logging.config=classpath:logback-nondefault.xml "); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.info("Hello world"); assertThat(this.output).contains("Hello world").doesNotContain("???").startsWith("null ").endsWith("BOOTBOOT"); } @@ -174,7 +178,7 @@ class LoggingApplicationListenerTests { void overrideConfigDoesNotExist() { addPropertiesToEnvironment(this.context, "logging.config=doesnotexist.xml"); assertThatIllegalStateException().isThrownBy( - () -> this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader())); + () -> this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader())); assertThat(this.output) .contains("Logging system failed to initialize using configuration from 'doesnotexist.xml'") .doesNotContain("JoranException"); @@ -184,7 +188,7 @@ class LoggingApplicationListenerTests { void azureDefaultLoggingConfigDoesNotCauseAFailure() { addPropertiesToEnvironment(this.context, "logging.config=-Djava.util.logging.config.file=\"d:\\home\\site\\wwwroot\\bin\\apache-tomcat-7.0.52\\conf\\logging.properties\""); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.info("Hello world"); assertThat(this.output).contains("Hello world").doesNotContain("???"); assertThat(new File(this.tempDir.toFile(), "/spring.log").exists()).isFalse(); @@ -193,7 +197,7 @@ class LoggingApplicationListenerTests { @Test void tomcatNopLoggingConfigDoesNotCauseAFailure() { addPropertiesToEnvironment(this.context, "LOGGING_CONFIG=-Dnop"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.info("Hello world"); assertThat(this.output).contains("Hello world").doesNotContain("???"); assertThat(new File(this.tempDir.toFile(), "/spring.log").exists()).isFalse(); @@ -203,7 +207,7 @@ class LoggingApplicationListenerTests { void overrideConfigBroken() { addPropertiesToEnvironment(this.context, "logging.config=classpath:logback-broken.xml"); assertThatIllegalStateException().isThrownBy(() -> { - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); assertThat(this.output).contains( "Logging system failed to initialize using configuration from 'classpath:logback-broken.xml'"); assertThat(this.output).contains("ConsolAppender"); @@ -214,7 +218,7 @@ class LoggingApplicationListenerTests { void addLogFileProperty() { addPropertiesToEnvironment(this.context, "logging.config=classpath:logback-nondefault.xml", "logging.file.name=" + this.logFile); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); Log logger = LogFactory.getLog(LoggingApplicationListenerTests.class); String existingOutput = this.output.toString(); logger.info("Hello world"); @@ -226,7 +230,7 @@ class LoggingApplicationListenerTests { void addLogFilePropertyWithDefault() { assertThat(this.logFile).doesNotExist(); addPropertiesToEnvironment(this.context, "logging.file.name=" + this.logFile); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); Log logger = LogFactory.getLog(LoggingApplicationListenerTests.class); logger.info("Hello world"); assertThat(this.logFile).isFile(); @@ -236,7 +240,7 @@ class LoggingApplicationListenerTests { void addLogPathProperty() { addPropertiesToEnvironment(this.context, "logging.config=classpath:logback-nondefault.xml", "logging.file.path=" + this.tempDir); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); Log logger = LogFactory.getLog(LoggingApplicationListenerTests.class); String existingOutput = this.output.toString(); logger.info("Hello world"); @@ -247,7 +251,7 @@ class LoggingApplicationListenerTests { @Test void parseDebugArg() { addPropertiesToEnvironment(this.context, "debug"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.debug("testatdebug"); this.logger.trace("testattrace"); assertThat(this.output).contains("testatdebug"); @@ -257,19 +261,19 @@ class LoggingApplicationListenerTests { @Test void parseDebugArgExpandGroups() { addPropertiesToEnvironment(this.context, "debug"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.loggerContext.getLogger("org.springframework.boot.actuate.endpoint.web").debug("testdebugwebgroup"); this.loggerContext.getLogger("org.hibernate.SQL").debug("testdebugsqlgroup"); assertThat(this.output).contains("testdebugwebgroup"); assertThat(this.output).contains("testdebugsqlgroup"); - LoggerGroups loggerGroups = (LoggerGroups) ReflectionTestUtils.getField(this.initializer, "loggerGroups"); + LoggerGroups loggerGroups = (LoggerGroups) ReflectionTestUtils.getField(this.listener, "loggerGroups"); assertThat(loggerGroups.get("web").getConfiguredLevel()).isEqualTo(LogLevel.DEBUG); } @Test void parseTraceArg() { addPropertiesToEnvironment(this.context, "trace"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.debug("testatdebug"); this.logger.trace("testattrace"); assertThat(this.output).contains("testatdebug"); @@ -288,7 +292,7 @@ class LoggingApplicationListenerTests { private void disableDebugTraceArg(String... environment) { addPropertiesToEnvironment(this.context, environment); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.debug("testatdebug"); this.logger.trace("testattrace"); assertThat(this.output).doesNotContain("testatdebug"); @@ -298,7 +302,7 @@ class LoggingApplicationListenerTests { @Test void parseLevels() { addPropertiesToEnvironment(this.context, "logging.level.org.springframework.boot=TRACE"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.debug("testatdebug"); this.logger.trace("testattrace"); assertThat(this.output).contains("testatdebug"); @@ -308,7 +312,7 @@ class LoggingApplicationListenerTests { @Test void parseLevelsCaseInsensitive() { addPropertiesToEnvironment(this.context, "logging.level.org.springframework.boot=TrAcE"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.debug("testatdebug"); this.logger.trace("testattrace"); assertThat(this.output).contains("testatdebug"); @@ -318,7 +322,7 @@ class LoggingApplicationListenerTests { @Test void parseLevelsTrimsWhitespace() { addPropertiesToEnvironment(this.context, "logging.level.org.springframework.boot= trace "); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.debug("testatdebug"); this.logger.trace("testattrace"); assertThat(this.output).contains("testatdebug"); @@ -328,7 +332,7 @@ class LoggingApplicationListenerTests { @Test void parseLevelsWithPlaceholder() { addPropertiesToEnvironment(this.context, "foo=TRACE", "logging.level.org.springframework.boot=${foo}"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.debug("testatdebug"); this.logger.trace("testattrace"); assertThat(this.output).contains("testatdebug"); @@ -340,13 +344,13 @@ class LoggingApplicationListenerTests { this.logger.setLevel(Level.INFO); addPropertiesToEnvironment(this.context, "logging.level.org.springframework.boot=GARBAGE"); assertThatExceptionOfType(BindException.class).isThrownBy( - () -> this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader())); + () -> this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader())); } @Test void parseLevelsNone() { addPropertiesToEnvironment(this.context, "logging.level.org.springframework.boot=OFF"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.debug("testatdebug"); this.logger.error("testaterror"); assertThat(this.output).doesNotContain("testatdebug").doesNotContain("testaterror"); @@ -355,7 +359,7 @@ class LoggingApplicationListenerTests { @Test void parseLevelsMapsFalseToOff() { addPropertiesToEnvironment(this.context, "logging.level.org.springframework.boot=false"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.debug("testatdebug"); this.logger.error("testaterror"); assertThat(this.output).doesNotContain("testatdebug").doesNotContain("testaterror"); @@ -363,20 +367,20 @@ class LoggingApplicationListenerTests { @Test void parseArgsDisabled() { - this.initializer.setParseArgs(false); + this.listener.setParseArgs(false); addPropertiesToEnvironment(this.context, "debug"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.debug("testatdebug"); assertThat(this.output).doesNotContain("testatdebug"); } @Test void parseArgsDoesntReplace() { - this.initializer.setSpringBootLogging(LogLevel.ERROR); - this.initializer.setParseArgs(false); + this.listener.setSpringBootLogging(LogLevel.ERROR); + this.listener.setParseArgs(false); multicastEvent(new ApplicationStartingEvent(this.bootstrapContext, this.springApplication, new String[] { "--debug" })); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.debug("testatdebug"); assertThat(this.output).doesNotContain("testatdebug"); } @@ -390,7 +394,7 @@ class LoggingApplicationListenerTests { @Test void defaultExceptionConversionWord() { - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.info("Hello world", new RuntimeException("Wrapper", new RuntimeException("Expected"))); assertThat(this.output).contains("Hello world"); assertThat(this.output).doesNotContain("Wrapped by: java.lang.RuntimeException: Wrapper"); @@ -399,7 +403,7 @@ class LoggingApplicationListenerTests { @Test void overrideExceptionConversionWord() { addPropertiesToEnvironment(this.context, "logging.exceptionConversionWord=%rEx"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.info("Hello world", new RuntimeException("Wrapper", new RuntimeException("Expected"))); assertThat(this.output).contains("Hello world"); assertThat(this.output).contains("Wrapped by: java.lang.RuntimeException: Wrapper"); @@ -436,8 +440,8 @@ class LoggingApplicationListenerTests { void closingContextCleansUpLoggingSystem() { System.setProperty(LoggingSystem.SYSTEM_PROPERTY, TestCleanupLoggingSystem.class.getName()); multicastEvent(new ApplicationStartingEvent(this.bootstrapContext, this.springApplication, new String[0])); - TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils - .getField(this.initializer, "loggingSystem"); + TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils.getField(this.listener, + "loggingSystem"); assertThat(loggingSystem.cleanedUp).isFalse(); multicastEvent(new ContextClosedEvent(this.context)); assertThat(loggingSystem.cleanedUp).isTrue(); @@ -447,8 +451,8 @@ class LoggingApplicationListenerTests { void closingChildContextDoesNotCleanUpLoggingSystem() { System.setProperty(LoggingSystem.SYSTEM_PROPERTY, TestCleanupLoggingSystem.class.getName()); multicastEvent(new ApplicationStartingEvent(this.bootstrapContext, this.springApplication, new String[0])); - TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils - .getField(this.initializer, "loggingSystem"); + TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils.getField(this.listener, + "loggingSystem"); assertThat(loggingSystem.cleanedUp).isFalse(); GenericApplicationContext childContext = new GenericApplicationContext(); childContext.setParent(this.context); @@ -465,7 +469,7 @@ class LoggingApplicationListenerTests { "logging.file.name=" + this.logFile, "logging.file.path=path", "logging.pattern.console=console", "logging.pattern.file=file", "logging.pattern.level=level", "logging.pattern.rolling-file-name=my.log.%d{yyyyMMdd}.%i.gz"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); assertThat(System.getProperty(LoggingSystemProperties.CONSOLE_LOG_PATTERN)).isEqualTo("console"); assertThat(System.getProperty(LoggingSystemProperties.FILE_LOG_PATTERN)).isEqualTo("file"); assertThat(System.getProperty(LoggingSystemProperties.EXCEPTION_CONVERSION_WORD)).isEqualTo("conversion"); @@ -479,7 +483,7 @@ class LoggingApplicationListenerTests { void environmentPropertiesIgnoreUnresolvablePlaceholders() { // gh-7719 addPropertiesToEnvironment(this.context, "logging.pattern.console=console ${doesnotexist}"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); assertThat(System.getProperty(LoggingSystemProperties.CONSOLE_LOG_PATTERN)) .isEqualTo("console ${doesnotexist}"); } @@ -487,7 +491,7 @@ class LoggingApplicationListenerTests { @Test void environmentPropertiesResolvePlaceholders() { addPropertiesToEnvironment(this.context, "logging.pattern.console=console ${pid}"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); assertThat(System.getProperty(LoggingSystemProperties.CONSOLE_LOG_PATTERN)) .isEqualTo(this.context.getEnvironment().getProperty("logging.pattern.console")); } @@ -495,7 +499,7 @@ class LoggingApplicationListenerTests { @Test void logFilePropertiesCanReferenceSystemProperties() { addPropertiesToEnvironment(this.context, "logging.file.name=" + this.tempDir + "${PID}.log"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); assertThat(System.getProperty(LoggingSystemProperties.LOG_FILE)) .isEqualTo(this.tempDir + new ApplicationPid().toString() + ".log"); } @@ -504,21 +508,44 @@ class LoggingApplicationListenerTests { void applicationFailedEventCleansUpLoggingSystem() { System.setProperty(LoggingSystem.SYSTEM_PROPERTY, TestCleanupLoggingSystem.class.getName()); multicastEvent(new ApplicationStartingEvent(this.bootstrapContext, this.springApplication, new String[0])); - TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils - .getField(this.initializer, "loggingSystem"); + TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils.getField(this.listener, + "loggingSystem"); assertThat(loggingSystem.cleanedUp).isFalse(); multicastEvent(new ApplicationFailedEvent(this.springApplication, new String[0], new GenericApplicationContext(), new Exception())); assertThat(loggingSystem.cleanedUp).isTrue(); } + @Test + void cleanupOccursAfterWebServerShutdown() { + System.setProperty(LoggingSystem.SYSTEM_PROPERTY, TestCleanupLoggingSystem.class.getName()); + this.springApplication.setWebApplicationType(WebApplicationType.NONE); + ConfigurableApplicationContext context = this.springApplication.run(); + ApplicationListener listener = this.springApplication.getListeners().stream() + .filter(LoggingApplicationListener.class::isInstance).findFirst().get(); + TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils.getField(listener, + "loggingSystem"); + assertThat(loggingSystem.cleanedUp).isFalse(); + WebServerStyleLifecycle lifecycle = context.getBean(WebServerStyleLifecycle.class); + AtomicBoolean called = new AtomicBoolean(); + AtomicBoolean cleanupOnStop = new AtomicBoolean(); + lifecycle.onStop = () -> { + called.set(true); + cleanupOnStop.set(loggingSystem.cleanedUp); + }; + context.close(); + assertThat(called).isTrue(); + assertThat(cleanupOnStop).isFalse(); + assertThat(loggingSystem.cleanedUp).isTrue(); + } + @Test void lowPriorityPropertySourceShouldNotOverrideRootLoggerConfig() { MutablePropertySources propertySources = this.context.getEnvironment().getPropertySources(); propertySources .addFirst(new MapPropertySource("test1", Collections.singletonMap("logging.level.ROOT", "DEBUG"))); propertySources.addLast(new MapPropertySource("test2", Collections.singletonMap("logging.level.root", "WARN"))); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); this.logger.debug("testatdebug"); assertThat(this.output).contains("testatdebug"); } @@ -526,7 +553,7 @@ class LoggingApplicationListenerTests { @Test void loggingGroupsDefaultsAreApplied() { addPropertiesToEnvironment(this.context, "logging.level.web=TRACE"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); assertTraceEnabled("org.springframework.core", false); assertTraceEnabled("org.springframework.core.codec", true); assertTraceEnabled("org.springframework.http", true); @@ -539,7 +566,7 @@ class LoggingApplicationListenerTests { void loggingGroupsCanBeDefined() { addPropertiesToEnvironment(this.context, "logging.group.foo=com.foo.bar,com.foo.baz", "logging.level.foo=TRACE"); - this.initializer.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + this.listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); assertTraceEnabled("com.foo", false); assertTraceEnabled("com.foo.bar", true); assertTraceEnabled("com.foo.baz", true); @@ -550,7 +577,7 @@ class LoggingApplicationListenerTests { } private void multicastEvent(ApplicationEvent event) { - multicastEvent(this.initializer, event); + multicastEvent(this.listener, event); } private void multicastEvent(ApplicationListener listener, ApplicationEvent event) { @@ -668,4 +695,39 @@ class LoggingApplicationListenerTests { } + @Configuration + @Import(WebServerStyleLifecycle.class) + static class TestConfiguration { + + } + + static class WebServerStyleLifecycle implements SmartLifecycle { + + private volatile boolean running; + + Runnable onStop; + + @Override + public void start() { + this.running = true; + } + + @Override + public void stop() { + this.running = false; + this.onStop.run(); + } + + @Override + public boolean isRunning() { + return this.running; + } + + @Override + public int getPhase() { + return Integer.MAX_VALUE - 1; + } + + } + }