From 00b668b2c52b8e121ed0cbb4d7956b340f3bd1ea Mon Sep 17 00:00:00 2001
From: Andy Wilkinson <awilkinson@pivotal.io>
Date: Thu, 3 Dec 2015 14:01:32 +0000
Subject: [PATCH] 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
---
 .../logging/LoggingApplicationListener.java   |  3 +-
 .../LoggingApplicationListenerTests.java      | 64 +++++++++++++++++++
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java b/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java
index 2d8c69b1e1..3fe1f4dbf5 100644
--- a/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java
+++ b/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java
@@ -145,7 +145,8 @@ public class LoggingApplicationListener implements SmartApplicationListener {
 			onApplicationEnvironmentPreparedEvent(
 					(ApplicationEnvironmentPreparedEvent) event);
 		}
-		else if (event instanceof ContextClosedEvent) {
+		else if (event instanceof ContextClosedEvent && ((ContextClosedEvent) event)
+				.getApplicationContext().getParent() == null) {
 			onContextClosedEvent();
 		}
 	}
diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/LoggingApplicationListenerTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/LoggingApplicationListenerTests.java
index d4bf7d11a1..4f77af2f0d 100644
--- a/spring-boot/src/test/java/org/springframework/boot/logging/LoggingApplicationListenerTests.java
+++ b/spring-boot/src/test/java/org/springframework/boot/logging/LoggingApplicationListenerTests.java
@@ -39,8 +39,10 @@ import org.springframework.boot.test.EnvironmentTestUtils;
 import org.springframework.boot.test.OutputCapture;
 import org.springframework.context.event.ContextClosedEvent;
 import org.springframework.context.support.GenericApplicationContext;
+import org.springframework.test.util.ReflectionTestUtils;
 
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
@@ -86,6 +88,7 @@ public class LoggingApplicationListenerTests {
 		System.clearProperty("LOG_FILE");
 		System.clearProperty("LOG_PATH");
 		System.clearProperty("PID");
+		System.clearProperty(LoggingSystem.SYSTEM_PROPERTY);
 		if (this.context != null) {
 			this.context.close();
 		}
@@ -302,6 +305,37 @@ public class LoggingApplicationListenerTests {
 		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() {
 		Logger rootLogger = LogManager.getLogManager().getLogger("");
 		Handler[] handlers = rootLogger.getHandlers();
@@ -312,4 +346,34 @@ public class LoggingApplicationListenerTests {
 		}
 		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;
+		}
+
+	}
 }