Clean up the logging system later in context close processing

Previously, the logging system was cleaned up in response to the
root context's ContextClosedEvent being received. This event is
published early in a context's close processing. As a result, the
logging system is in cleaned up state while, for example, disposable
beans are being destroyed.

This commit reworks the logic that triggers logging system clean up
to use a disposable bean instead. Disposable beans are called in
reverse-registration order. The logging clean up bean is registered as
early as possible so that it should be the last disposable bean to
be called.

Closes gh-11676

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/BackgroundPreinitializer.java

@@ -30,7 +30,7 @@ import org.springframework.boot.context.event.ApplicationFailedEvent;
 import org.springframework.boot.context.event.ApplicationReadyEvent;
 import org.springframework.boot.context.event.ApplicationStartingEvent;
 import org.springframework.boot.context.event.SpringApplicationEvent;
-import org.springframework.boot.context.logging.LoggingApplicationListener;
+import org.springframework.boot.context.logging.LoggingSystemLifecycle;
 import org.springframework.context.ApplicationListener;
 import org.springframework.core.annotation.Order;
 import org.springframework.format.support.DefaultFormattingConversionService;
@@ -45,7 +45,7 @@ import org.springframework.http.converter.support.AllEncompassingFormHttpMessage
  * @author Andy Wilkinson
  * @since 1.3.0
  */
-@Order(LoggingApplicationListener.DEFAULT_ORDER + 1)
+@Order(LoggingSystemLifecycle.DEFAULT_ORDER + 1)
 public class BackgroundPreinitializer
 		implements ApplicationListener<SpringApplicationEvent> {
 

spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/RemoteSpringApplication.java

 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 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.
@@ -27,7 +27,7 @@ import org.springframework.boot.WebApplicationType;
 import org.springframework.boot.context.config.AnsiOutputApplicationListener;
 import org.springframework.boot.context.config.ConfigFileApplicationListener;
 import org.springframework.boot.context.logging.ClasspathLoggingApplicationListener;
-import org.springframework.boot.context.logging.LoggingApplicationListener;
+import org.springframework.boot.context.logging.LoggingSystemLifecycle;
 import org.springframework.boot.devtools.remote.client.RemoteClientConfiguration;
 import org.springframework.boot.devtools.restart.RestartInitializer;
 import org.springframework.boot.devtools.restart.RestartScopeInitializer;
@@ -74,7 +74,7 @@ public final class RemoteSpringApplication {
 		listeners.add(new AnsiOutputApplicationListener());
 		listeners.add(new ConfigFileApplicationListener());
 		listeners.add(new ClasspathLoggingApplicationListener());
-		listeners.add(new LoggingApplicationListener());
+		listeners.add(new LoggingSystemLifecycle());
 		listeners.add(new RemoteUrlPropertyExtractor());
 		return listeners;
 	}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/ClasspathLoggingApplicationListener.java

 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 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.
@@ -41,7 +41,7 @@ import org.springframework.core.ResolvableType;
 public final class ClasspathLoggingApplicationListener
 		implements GenericApplicationListener {
 
-	private static final int ORDER = LoggingApplicationListener.DEFAULT_ORDER + 1;
+	private static final int ORDER = LoggingSystemLifecycle.DEFAULT_ORDER + 1;
 
 	private static final Log logger = LogFactory
 			.getLog(ClasspathLoggingApplicationListener.class);

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java → spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingSystemLifecycle.java

 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 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.
@@ -24,7 +24,12 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.beans.factory.BeanFactory;
+import org.springframework.beans.factory.DisposableBean;
+import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
+import org.springframework.beans.factory.support.BeanDefinitionBuilder;
+import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent;
 import org.springframework.boot.context.event.ApplicationFailedEvent;
@@ -38,8 +43,9 @@ import org.springframework.boot.logging.LoggingInitializationContext;
 import org.springframework.boot.logging.LoggingSystem;
 import org.springframework.boot.logging.LoggingSystemProperties;
 import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContextInitializer;
 import org.springframework.context.ApplicationEvent;
-import org.springframework.context.ApplicationListener;
+import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.event.ContextClosedEvent;
 import org.springframework.context.event.GenericApplicationListener;
 import org.springframework.core.Ordered;
@@ -52,10 +58,11 @@ import org.springframework.util.ResourceUtils;
 import org.springframework.util.StringUtils;
 
 /**
- * An {@link ApplicationListener} that configures the {@link LoggingSystem}. If the
- * environment contains a {@code logging.config} property it will be used to bootstrap the
- * logging system, otherwise a default configuration is used. Regardless, logging levels
- * will be customized if the environment contains {@code logging.level.*} entries.
+ * {@code LoggingSystemLifecyle} configures the {@link LoggingSystem} and manages its
+ * lifecycle. If the environment contains a {@code logging.config} property it will be
+ * used to bootstrap the logging system, otherwise a default configuration is used.
+ * Regardless, logging levels will be customized if the environment contains
+ * {@code logging.level.*} entries.
  * <p>
  * Debug and trace logging for Spring, Tomcat, Jetty and Hibernate will be enabled when
  * the environment contains {@code debug} or {@code trace} properties that aren't set to
@@ -82,7 +89,8 @@ import org.springframework.util.StringUtils;
  * @since 2.0.0
  * @see LoggingSystem#get(ClassLoader)
  */
-public class LoggingApplicationListener implements GenericApplicationListener {
+public class LoggingSystemLifecycle implements GenericApplicationListener,
+		ApplicationContextInitializer<ConfigurableApplicationContext>, Ordered {
 
 	private static final Bindable<Map<String, String>> STRING_STRING_MAP = Bindable
 			.mapOf(String.class, String.class);
@@ -175,10 +183,6 @@ 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 ApplicationFailedEvent) {
 			onApplicationFailedEvent();
 		}
@@ -207,12 +211,6 @@ public class LoggingApplicationListener implements GenericApplicationListener {
 		}
 	}
 
-	private void onContextClosedEvent() {
-		if (this.loggingSystem != null) {
-			this.loggingSystem.cleanUp();
-		}
-	}
-
 	private void onApplicationFailedEvent() {
 		if (this.loggingSystem != null) {
 			this.loggingSystem.cleanUp();
@@ -367,4 +365,41 @@ public class LoggingApplicationListener implements GenericApplicationListener {
 		this.parseArgs = parseArgs;
 	}
 
+	@Override
+	public void initialize(ConfigurableApplicationContext applicationContext) {
+		registerCleanupBean(applicationContext);
+	}
+
+	private void registerCleanupBean(ConfigurableApplicationContext applicationContext) {
+		BeanFactory beanFactory = applicationContext.getBeanFactory();
+		BeanDefinition beanDefinition = BeanDefinitionBuilder
+				.genericBeanDefinition(LoggingSystemCleanup.class)
+				.addConstructorArgValue(this.loggingSystem)
+				.addConstructorArgValue(applicationContext).getBeanDefinition();
+		((BeanDefinitionRegistry) beanFactory).registerBeanDefinition(
+				LoggingSystemCleanup.class.getName(), beanDefinition);
+	}
+
+	private static final class LoggingSystemCleanup implements DisposableBean {
+
+		private final LoggingSystem loggingSystem;
+
+		private final ApplicationContext applicationContext;
+
+		private LoggingSystemCleanup(LoggingSystem loggingSystem,
+				ApplicationContext applicationContext) {
+			this.loggingSystem = loggingSystem;
+			this.applicationContext = applicationContext;
+		}
+
+		@Override
+		public void destroy() throws Exception {
+			if (this.applicationContext.getParent() == null
+					&& this.loggingSystem != null) {
+				this.loggingSystem.cleanUp();
+			}
+		}
+
+	}
+
 }

spring-boot-project/spring-boot/src/main/resources/META-INF/spring.factories

@@ -16,6 +16,7 @@ org.springframework.context.ApplicationContextInitializer=\
 org.springframework.boot.context.ConfigurationWarningsApplicationContextInitializer,\
 org.springframework.boot.context.ContextIdApplicationContextInitializer,\
 org.springframework.boot.context.config.DelegatingApplicationContextInitializer,\
+org.springframework.boot.context.logging.LoggingSystemLifecycle,\
 org.springframework.boot.web.context.ServerPortInfoApplicationContextInitializer
 
 # Application Listeners
@@ -27,7 +28,7 @@ org.springframework.boot.context.config.AnsiOutputApplicationListener,\
 org.springframework.boot.context.config.ConfigFileApplicationListener,\
 org.springframework.boot.context.config.DelegatingApplicationListener,\
 org.springframework.boot.context.logging.ClasspathLoggingApplicationListener,\
-org.springframework.boot.context.logging.LoggingApplicationListener,\
+org.springframework.boot.context.logging.LoggingSystemLifecycle,\
 org.springframework.boot.liquibase.LiquibaseServiceLocatorApplicationListener
 
 # Environment Post Processors

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/builder/SpringApplicationBuilderTests.java

@@ -277,7 +277,7 @@ public class SpringApplicationBuilderTests {
 		SpringApplicationBuilder application = new SpringApplicationBuilder(
 				ExampleConfig.class).web(WebApplicationType.NONE);
 		this.context = application.run();
-		assertThat(application.application().getInitializers()).hasSize(4);
+		assertThat(application.application().getInitializers()).hasSize(5);
 	}
 
 	@Test
@@ -286,7 +286,7 @@ public class SpringApplicationBuilderTests {
 				ExampleConfig.class).child(ChildConfig.class)
 						.web(WebApplicationType.NONE);
 		this.context = application.run();
-		assertThat(application.application().getInitializers()).hasSize(5);
+		assertThat(application.application().getInitializers()).hasSize(6);
 	}
 
 	@Test
@@ -296,7 +296,7 @@ public class SpringApplicationBuilderTests {
 						(ConfigurableApplicationContext applicationContext) -> {
 						});
 		this.context = application.run();
-		assertThat(application.application().getInitializers()).hasSize(5);
+		assertThat(application.application().getInitializers()).hasSize(6);
 	}
 
 	@Test

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerIntegrationTests.java → spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingSystemLifecycleIntegrationTests.java

 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 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.
@@ -33,11 +33,11 @@ import org.springframework.stereotype.Component;
 import static org.assertj.core.api.Assertions.assertThat;
 
 /**
- * Integration tests for {@link LoggingApplicationListener}.
+ * Integration tests for {@link LoggingSystemLifecycle}.
  *
  * @author Stephane Nicoll
  */
-public class LoggingApplicationListenerIntegrationTests {
+public class LoggingSystemLifecycleIntegrationTests {
 
 	@Rule
 	public OutputCapture outputCapture = new OutputCapture();

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java → spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingSystemLifecycleTests.java

 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 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.
@@ -56,7 +56,6 @@ import org.springframework.boot.testsupport.runner.classpath.ClassPathExclusions
 import org.springframework.boot.testsupport.runner.classpath.ModifiedClassPathRunner;
 import org.springframework.context.ApplicationEvent;
 import org.springframework.context.ApplicationListener;
-import org.springframework.context.event.ContextClosedEvent;
 import org.springframework.context.event.SimpleApplicationEventMulticaster;
 import org.springframework.context.support.GenericApplicationContext;
 import org.springframework.core.env.ConfigurableEnvironment;
@@ -70,7 +69,7 @@ import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.not;
 
 /**
- * Tests for {@link LoggingApplicationListener} with Logback.
+ * Tests for {@link LoggingSystemLifecycle} with Logback.
  *
  * @author Dave Syer
  * @author Phillip Webb
@@ -80,7 +79,7 @@ import static org.hamcrest.Matchers.not;
  */
 @RunWith(ModifiedClassPathRunner.class)
 @ClassPathExclusions("log4j*.jar")
-public class LoggingApplicationListenerTests {
+public class LoggingSystemLifecycleTests {
 
 	private static final String[] NO_ARGS = {};
 
@@ -93,7 +92,7 @@ public class LoggingApplicationListenerTests {
 	@Rule
 	public OutputCapture outputCapture = new OutputCapture();
 
-	private final LoggingApplicationListener initializer = new LoggingApplicationListener();
+	private final LoggingSystemLifecycle initializer = new LoggingSystemLifecycle();
 
 	private final Log logger = new SLF4JLogFactory().getInstance(getClass());
 
@@ -218,7 +217,7 @@ public class LoggingApplicationListenerTests {
 				"logging.file=target/foo.log");
 		this.initializer.initialize(this.context.getEnvironment(),
 				this.context.getClassLoader());
-		Log logger = LogFactory.getLog(LoggingApplicationListenerTests.class);
+		Log logger = LogFactory.getLog(LoggingSystemLifecycleTests.class);
 		logger.info("Hello world");
 		String output = this.outputCapture.toString().trim();
 		assertThat(output).startsWith("target/foo.log");
@@ -231,7 +230,7 @@ public class LoggingApplicationListenerTests {
 				"logging.file=target/foo.log");
 		this.initializer.initialize(this.context.getEnvironment(),
 				this.context.getClassLoader());
-		Log logger = LogFactory.getLog(LoggingApplicationListenerTests.class);
+		Log logger = LogFactory.getLog(LoggingSystemLifecycleTests.class);
 		logger.info("Hello world");
 		assertThat(new File("target/foo.log").exists()).isTrue();
 	}
@@ -243,7 +242,7 @@ public class LoggingApplicationListenerTests {
 				"logging.path=target/foo/");
 		this.initializer.initialize(this.context.getEnvironment(),
 				this.context.getClassLoader());
-		Log logger = LogFactory.getLog(LoggingApplicationListenerTests.class);
+		Log logger = LogFactory.getLog(LoggingSystemLifecycleTests.class);
 		logger.info("Hello world");
 		String output = this.outputCapture.toString().trim();
 		assertThat(output).startsWith("target/foo/spring.log");
@@ -387,8 +386,10 @@ public class LoggingApplicationListenerTests {
 
 	@Test
 	public void bridgeHandlerLifecycle() {
+		this.initializer.initialize(this.context);
+		this.context.refresh();
 		assertThat(bridgeHandlerInstalled()).isTrue();
-		multicastEvent(new ContextClosedEvent(this.context));
+		this.context.close();
 		assertThat(bridgeHandlerInstalled()).isFalse();
 	}
 
@@ -449,10 +450,12 @@ public class LoggingApplicationListenerTests {
 				TestCleanupLoggingSystem.class.getName());
 		multicastEvent(
 				new ApplicationStartingEvent(this.springApplication, new String[0]));
+		this.initializer.initialize(this.context);
+		this.context.refresh();
 		TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils
 				.getField(this.initializer, "loggingSystem");
 		assertThat(loggingSystem.cleanedUp).isFalse();
-		multicastEvent(new ContextClosedEvent(this.context));
+		this.context.close();
 		assertThat(loggingSystem.cleanedUp).isTrue();
 	}
 
@@ -462,16 +465,19 @@ public class LoggingApplicationListenerTests {
 				TestCleanupLoggingSystem.class.getName());
 		multicastEvent(
 				new ApplicationStartingEvent(this.springApplication, new String[0]));
+		this.initializer.initialize(this.context);
+		this.context.refresh();
 		TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils
 				.getField(this.initializer, "loggingSystem");
 		assertThat(loggingSystem.cleanedUp).isFalse();
 		GenericApplicationContext childContext = new GenericApplicationContext();
 		childContext.setParent(this.context);
-		multicastEvent(new ContextClosedEvent(childContext));
+		this.initializer.initialize(childContext);
+		childContext.refresh();
+		childContext.close();
 		assertThat(loggingSystem.cleanedUp).isFalse();
-		multicastEvent(new ContextClosedEvent(this.context));
+		this.context.close();
 		assertThat(loggingSystem.cleanedUp).isTrue();
-		childContext.close();
 	}
 
 	@Test
@@ -613,8 +619,7 @@ public class LoggingApplicationListenerTests {
 
 	}
 
-	public static class TestLoggingApplicationListener
-			extends LoggingApplicationListener {
+	public static class TestLoggingApplicationListener extends LoggingSystemLifecycle {
 
 		private Thread shutdownHook;