Revert "Clean up the logging system later in context close processing"

This reverts commit 8619256d2a.

See gh-11676
This commit is contained in:
Andy Wilkinson 2018-01-25 13:15:19 +00:00
parent d65f9b25bc
commit cedb6b2f17
8 changed files with 46 additions and 87 deletions

View File

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

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2012-2018 the original author or authors. * Copyright 2012-2017 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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.AnsiOutputApplicationListener;
import org.springframework.boot.context.config.ConfigFileApplicationListener; import org.springframework.boot.context.config.ConfigFileApplicationListener;
import org.springframework.boot.context.logging.ClasspathLoggingApplicationListener; import org.springframework.boot.context.logging.ClasspathLoggingApplicationListener;
import org.springframework.boot.context.logging.LoggingSystemLifecycle; import org.springframework.boot.context.logging.LoggingApplicationListener;
import org.springframework.boot.devtools.remote.client.RemoteClientConfiguration; import org.springframework.boot.devtools.remote.client.RemoteClientConfiguration;
import org.springframework.boot.devtools.restart.RestartInitializer; import org.springframework.boot.devtools.restart.RestartInitializer;
import org.springframework.boot.devtools.restart.RestartScopeInitializer; import org.springframework.boot.devtools.restart.RestartScopeInitializer;
@ -74,7 +74,7 @@ public final class RemoteSpringApplication {
listeners.add(new AnsiOutputApplicationListener()); listeners.add(new AnsiOutputApplicationListener());
listeners.add(new ConfigFileApplicationListener()); listeners.add(new ConfigFileApplicationListener());
listeners.add(new ClasspathLoggingApplicationListener()); listeners.add(new ClasspathLoggingApplicationListener());
listeners.add(new LoggingSystemLifecycle()); listeners.add(new LoggingApplicationListener());
listeners.add(new RemoteUrlPropertyExtractor()); listeners.add(new RemoteUrlPropertyExtractor());
return listeners; return listeners;
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2012-2018 the original author or authors. * Copyright 2012-2017 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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 public final class ClasspathLoggingApplicationListener
implements GenericApplicationListener { implements GenericApplicationListener {
private static final int ORDER = LoggingSystemLifecycle.DEFAULT_ORDER + 1; private static final int ORDER = LoggingApplicationListener.DEFAULT_ORDER + 1;
private static final Log logger = LogFactory private static final Log logger = LogFactory
.getLog(ClasspathLoggingApplicationListener.class); .getLog(ClasspathLoggingApplicationListener.class);

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2012-2018 the original author or authors. * Copyright 2012-2017 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -24,12 +24,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.commons.logging.Log; import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory; 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.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.SpringApplication;
import org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent; import org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent;
import org.springframework.boot.context.event.ApplicationFailedEvent; import org.springframework.boot.context.event.ApplicationFailedEvent;
@ -43,9 +38,8 @@ import org.springframework.boot.logging.LoggingInitializationContext;
import org.springframework.boot.logging.LoggingSystem; import org.springframework.boot.logging.LoggingSystem;
import org.springframework.boot.logging.LoggingSystemProperties; import org.springframework.boot.logging.LoggingSystemProperties;
import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextInitializer;
import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationEvent;
import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.ApplicationListener;
import org.springframework.context.event.ContextClosedEvent; import org.springframework.context.event.ContextClosedEvent;
import org.springframework.context.event.GenericApplicationListener; import org.springframework.context.event.GenericApplicationListener;
import org.springframework.core.Ordered; import org.springframework.core.Ordered;
@ -58,11 +52,10 @@ import org.springframework.util.ResourceUtils;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
/** /**
* {@code LoggingSystemLifecyle} configures the {@link LoggingSystem} and manages its * An {@link ApplicationListener} that configures the {@link LoggingSystem}. If the
* lifecycle. If the environment contains a {@code logging.config} property it will be * environment contains a {@code logging.config} property it will be used to bootstrap the
* used to bootstrap the logging system, otherwise a default configuration is used. * logging system, otherwise a default configuration is used. Regardless, logging levels
* Regardless, logging levels will be customized if the environment contains * will be customized if the environment contains {@code logging.level.*} entries.
* {@code logging.level.*} entries.
* <p> * <p>
* Debug and trace logging for Spring, Tomcat, Jetty and Hibernate will be enabled when * 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 * the environment contains {@code debug} or {@code trace} properties that aren't set to
@ -89,8 +82,7 @@ import org.springframework.util.StringUtils;
* @since 2.0.0 * @since 2.0.0
* @see LoggingSystem#get(ClassLoader) * @see LoggingSystem#get(ClassLoader)
*/ */
public class LoggingSystemLifecycle implements GenericApplicationListener, public class LoggingApplicationListener implements GenericApplicationListener {
ApplicationContextInitializer<ConfigurableApplicationContext>, Ordered {
private static final Bindable<Map<String, String>> STRING_STRING_MAP = Bindable private static final Bindable<Map<String, String>> STRING_STRING_MAP = Bindable
.mapOf(String.class, String.class); .mapOf(String.class, String.class);
@ -183,6 +175,10 @@ public class LoggingSystemLifecycle implements GenericApplicationListener,
else if (event instanceof ApplicationPreparedEvent) { else if (event instanceof ApplicationPreparedEvent) {
onApplicationPreparedEvent((ApplicationPreparedEvent) event); onApplicationPreparedEvent((ApplicationPreparedEvent) event);
} }
else if (event instanceof ContextClosedEvent && ((ContextClosedEvent) event)
.getApplicationContext().getParent() == null) {
onContextClosedEvent();
}
else if (event instanceof ApplicationFailedEvent) { else if (event instanceof ApplicationFailedEvent) {
onApplicationFailedEvent(); onApplicationFailedEvent();
} }
@ -211,6 +207,12 @@ public class LoggingSystemLifecycle implements GenericApplicationListener,
} }
} }
private void onContextClosedEvent() {
if (this.loggingSystem != null) {
this.loggingSystem.cleanUp();
}
}
private void onApplicationFailedEvent() { private void onApplicationFailedEvent() {
if (this.loggingSystem != null) { if (this.loggingSystem != null) {
this.loggingSystem.cleanUp(); this.loggingSystem.cleanUp();
@ -365,41 +367,4 @@ public class LoggingSystemLifecycle implements GenericApplicationListener,
this.parseArgs = parseArgs; 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();
}
}
}
} }

View File

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

View File

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

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2012-2018 the original author or authors. * Copyright 2012-2017 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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; import static org.assertj.core.api.Assertions.assertThat;
/** /**
* Integration tests for {@link LoggingSystemLifecycle}. * Integration tests for {@link LoggingApplicationListener}.
* *
* @author Stephane Nicoll * @author Stephane Nicoll
*/ */
public class LoggingSystemLifecycleIntegrationTests { public class LoggingApplicationListenerIntegrationTests {
@Rule @Rule
public OutputCapture outputCapture = new OutputCapture(); public OutputCapture outputCapture = new OutputCapture();

View File

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