Commit 00b668b2 authored by Andy Wilkinson's avatar Andy Wilkinson

Only clean up logging system when root application context is closed

Previously, LoggingApplicationListener would clean up the logging
system in response to any application context with which it was
registered being closed. This caused problems when a child context was
closed. Specifically, closing the child context would cause any
SLF4J-based logging systems to unregister the JUL bridge handler
preventing an JUL logging being bridged into Logback or Log4J2.

This commit updates LoggingApplicationListener so that the logging
system is only cleaned up when a root application context is
closed.

Closes gh-4651
parent d6bd120b
...@@ -145,7 +145,8 @@ public class LoggingApplicationListener implements SmartApplicationListener { ...@@ -145,7 +145,8 @@ public class LoggingApplicationListener implements SmartApplicationListener {
onApplicationEnvironmentPreparedEvent( onApplicationEnvironmentPreparedEvent(
(ApplicationEnvironmentPreparedEvent) event); (ApplicationEnvironmentPreparedEvent) event);
} }
else if (event instanceof ContextClosedEvent) { else if (event instanceof ContextClosedEvent && ((ContextClosedEvent) event)
.getApplicationContext().getParent() == null) {
onContextClosedEvent(); onContextClosedEvent();
} }
} }
......
...@@ -39,8 +39,10 @@ import org.springframework.boot.test.EnvironmentTestUtils; ...@@ -39,8 +39,10 @@ import org.springframework.boot.test.EnvironmentTestUtils;
import org.springframework.boot.test.OutputCapture; import org.springframework.boot.test.OutputCapture;
import org.springframework.context.event.ContextClosedEvent; import org.springframework.context.event.ContextClosedEvent;
import org.springframework.context.support.GenericApplicationContext; import org.springframework.context.support.GenericApplicationContext;
import org.springframework.test.util.ReflectionTestUtils;
import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
...@@ -86,6 +88,7 @@ public class LoggingApplicationListenerTests { ...@@ -86,6 +88,7 @@ public class LoggingApplicationListenerTests {
System.clearProperty("LOG_FILE"); System.clearProperty("LOG_FILE");
System.clearProperty("LOG_PATH"); System.clearProperty("LOG_PATH");
System.clearProperty("PID"); System.clearProperty("PID");
System.clearProperty(LoggingSystem.SYSTEM_PROPERTY);
if (this.context != null) { if (this.context != null) {
this.context.close(); this.context.close();
} }
...@@ -302,6 +305,37 @@ public class LoggingApplicationListenerTests { ...@@ -302,6 +305,37 @@ public class LoggingApplicationListenerTests {
assertFalse(bridgeHandlerInstalled()); assertFalse(bridgeHandlerInstalled());
} }
@Test
public void closingContextCleansUpLoggingSystem() {
System.setProperty(LoggingSystem.SYSTEM_PROPERTY,
TestCleanupLoggingSystem.class.getName());
this.initializer.onApplicationEvent(
new ApplicationStartedEvent(this.springApplication, new String[0]));
TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils
.getField(this.initializer, "loggingSystem");
assertThat(loggingSystem.cleanedUp, is(false));
this.initializer.onApplicationEvent(new ContextClosedEvent(this.context));
assertThat(loggingSystem.cleanedUp, is(true));
}
@Test
public void closingChildContextDoesNotCleanUpLoggingSystem() {
System.setProperty(LoggingSystem.SYSTEM_PROPERTY,
TestCleanupLoggingSystem.class.getName());
this.initializer.onApplicationEvent(
new ApplicationStartedEvent(this.springApplication, new String[0]));
TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils
.getField(this.initializer, "loggingSystem");
assertThat(loggingSystem.cleanedUp, is(false));
GenericApplicationContext childContext = new GenericApplicationContext();
childContext.setParent(this.context);
this.initializer.onApplicationEvent(new ContextClosedEvent(childContext));
assertThat(loggingSystem.cleanedUp, is(false));
this.initializer.onApplicationEvent(new ContextClosedEvent(this.context));
assertThat(loggingSystem.cleanedUp, is(true));
childContext.close();
}
private boolean bridgeHandlerInstalled() { private boolean bridgeHandlerInstalled() {
Logger rootLogger = LogManager.getLogManager().getLogger(""); Logger rootLogger = LogManager.getLogManager().getLogger("");
Handler[] handlers = rootLogger.getHandlers(); Handler[] handlers = rootLogger.getHandlers();
...@@ -312,4 +346,34 @@ public class LoggingApplicationListenerTests { ...@@ -312,4 +346,34 @@ public class LoggingApplicationListenerTests {
} }
return false; return false;
} }
static final class TestCleanupLoggingSystem extends LoggingSystem {
private boolean cleanedUp = false;
public TestCleanupLoggingSystem(ClassLoader classLoader) {
}
@Override
public void beforeInitialize() {
}
@Override
public void initialize(String configLocation, LogFile logFile) {
}
@Override
public void setLogLevel(String loggerName, LogLevel level) {
}
@Override
public void cleanUp() {
this.cleanedUp = true;
}
}
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment