Ensure logging filters are removed on cleanup

Update `Log4J2LoggingSystem` and `LogbackLoggingSystem` to ensure
that filters are removed when the `cleanUp` method is called. Prior
to this commit application failures would not remove the filter and
no log messages would appear.

The `LoggingApplicationListener` has also been updated since it
previously failed to handle `ApplicationFailureEvents`.

Finally `EventPublishingRunListener` and `DelegatingApplicationListener`
have been updated to deal with `null` parameters and to cope with
listener errors.

Fixes gh-7758

Author: philwebb

spring-boot/src/main/java/org/springframework/boot/SpringApplication.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2016 the original author or authors.
+ * Copyright 2012-2017 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.
@@ -1233,7 +1233,6 @@ public static int exit(ApplicationContext context,
 			finally {
 				close(context);
 			}
-
 		}
 		catch (Exception ex) {
 			ex.printStackTrace();

spring-boot/src/main/java/org/springframework/boot/context/config/DelegatingApplicationListener.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2014 the original author or authors.
+ * Copyright 2012-2017 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.
@@ -17,6 +17,7 @@
 package org.springframework.boot.context.config;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import org.springframework.beans.BeanUtils;
@@ -70,8 +71,11 @@ public void onApplicationEvent(ApplicationEvent event) {
 
 	@SuppressWarnings("unchecked")
 	private List<ApplicationListener<ApplicationEvent>> getListeners(
-			ConfigurableEnvironment env) {
-		String classNames = env.getProperty(PROPERTY_NAME);
+			ConfigurableEnvironment environment) {
+		if (environment == null) {
+			return Collections.emptyList();
+		}
+		String classNames = environment.getProperty(PROPERTY_NAME);
 		List<ApplicationListener<ApplicationEvent>> listeners = new ArrayList<ApplicationListener<ApplicationEvent>>();
 		if (StringUtils.hasLength(classNames)) {
 			for (String className : StringUtils.commaDelimitedListToSet(classNames)) {

spring-boot/src/main/java/org/springframework/boot/context/event/EventPublishingRunListener.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2016 the original author or authors.
+ * Copyright 2012-2017 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.
@@ -16,6 +16,9 @@
 
 package org.springframework.boot.context.event;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.SpringApplicationRunListener;
 import org.springframework.context.ApplicationContextAware;
@@ -25,6 +28,7 @@
 import org.springframework.context.event.SimpleApplicationEventMulticaster;
 import org.springframework.core.Ordered;
 import org.springframework.core.env.ConfigurableEnvironment;
+import org.springframework.util.ErrorHandler;
 
 /**
  * {@link SpringApplicationRunListener} to publish {@link SpringApplicationEvent}s.
@@ -41,7 +45,7 @@ public class EventPublishingRunListener implements SpringApplicationRunListener,
 
 	private final String[] args;
 
-	private final ApplicationEventMulticaster initialMulticaster;
+	private final SimpleApplicationEventMulticaster initialMulticaster;
 
 	public EventPublishingRunListener(SpringApplication application, String[] args) {
 		this.application = application;
@@ -88,9 +92,18 @@ public void contextLoaded(ConfigurableApplicationContext context) {
 
 	@Override
 	public void finished(ConfigurableApplicationContext context, Throwable exception) {
-		// Listeners have been registered to the application context so we should
-		// use it at this point
-		context.publishEvent(getFinishedEvent(context, exception));
+		SpringApplicationEvent event = getFinishedEvent(context, exception);
+		if (context != null) {
+			// Listeners have been registered to the application context so we should
+			// use it at this point if we can
+			context.publishEvent(event);
+		}
+		else {
+			if (event instanceof ApplicationFailedEvent) {
+				this.initialMulticaster.setErrorHandler(new LoggingErrorHandler());
+			}
+			this.initialMulticaster.multicastEvent(event);
+		}
 	}
 
 	private SpringApplicationEvent getFinishedEvent(
@@ -102,4 +115,15 @@ private SpringApplicationEvent getFinishedEvent(
 		return new ApplicationReadyEvent(this.application, this.args, context);
 	}
 
+	private static class LoggingErrorHandler implements ErrorHandler {
+
+		private static Log logger = LogFactory.getLog(EventPublishingRunListener.class);
+
+		@Override
+		public void handleError(Throwable throwable) {
+			logger.warn("Error calling ApplicationEventListener", throwable);
+		}
+
+	}
+
 }

spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2016 the original author or authors.
+ * Copyright 2012-2017 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.
@@ -163,7 +163,7 @@ public class LoggingApplicationListener implements GenericApplicationListener {
 
 	private static Class<?>[] EVENT_TYPES = { ApplicationStartedEvent.class,
 			ApplicationEnvironmentPreparedEvent.class, ApplicationPreparedEvent.class,
-			ContextClosedEvent.class };
+			ContextClosedEvent.class, ApplicationFailedEvent.class };
 
 	private static Class<?>[] SOURCE_TYPES = { SpringApplication.class,
 			ApplicationContext.class };

spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2016 the original author or authors.
+ * Copyright 2012-2017 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.
@@ -218,6 +218,7 @@ public void cleanUp() {
 		super.cleanUp();
 		LoggerContext loggerContext = getLoggerContext();
 		markAsUninitialized(loggerContext);
+		loggerContext.getConfiguration().removeFilter(FILTER);
 	}
 
 	private LoggerConfig getLoggerConfig(String name) {

spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2016 the original author or authors.
+ * Copyright 2012-2017 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.
@@ -193,9 +193,11 @@ private void addLevelChangePropagator(LoggerContext loggerContext) {
 
 	@Override
 	public void cleanUp() {
-		markAsUninitialized(getLoggerContext());
+		LoggerContext context = getLoggerContext();
+		markAsUninitialized(context);
 		super.cleanUp();
-		getLoggerContext().getStatusManager().clear();
+		context.getStatusManager().clear();
+		context.getTurboFilterList().remove(FILTER);
 	}
 
 	@Override

spring-boot/src/test/java/org/springframework/boot/logging/LoggingApplicationListenerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2016 the original author or authors.
+ * Copyright 2012-2017 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,10 @@
 import org.springframework.boot.context.event.ApplicationStartedEvent;
 import org.springframework.boot.logging.java.JavaLoggingSystem;
 import org.springframework.boot.testutil.InternalOutputCapture;
+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.test.context.support.TestPropertySourceUtils;
 import org.springframework.test.util.ReflectionTestUtils;
@@ -83,8 +86,7 @@ public class LoggingApplicationListenerTests {
 	public void init() throws SecurityException, IOException {
 		LogManager.getLogManager().readConfiguration(
 				JavaLoggingSystem.class.getResourceAsStream("logging.properties"));
-		this.initializer.onApplicationEvent(
-				new ApplicationStartedEvent(new SpringApplication(), NO_ARGS));
+		multicastEvent(new ApplicationStartedEvent(new SpringApplication(), NO_ARGS));
 		new File("target/foo.log").delete();
 		new File(tmpDir() + "/spring.log").delete();
 	}
@@ -352,8 +354,8 @@ public void parseArgsDisabled() throws Exception {
 	public void parseArgsDoesntReplace() throws Exception {
 		this.initializer.setSpringBootLogging(LogLevel.ERROR);
 		this.initializer.setParseArgs(false);
-		this.initializer.onApplicationEvent(new ApplicationStartedEvent(
-				this.springApplication, new String[] { "--debug" }));
+		multicastEvent(new ApplicationStartedEvent(this.springApplication,
+				new String[] { "--debug" }));
 		this.initializer.initialize(this.context.getEnvironment(),
 				this.context.getClassLoader());
 		this.logger.debug("testatdebug");
@@ -363,7 +365,7 @@ public void parseArgsDoesntReplace() throws Exception {
 	@Test
 	public void bridgeHandlerLifecycle() throws Exception {
 		assertThat(bridgeHandlerInstalled()).isTrue();
-		this.initializer.onApplicationEvent(new ContextClosedEvent(this.context));
+		multicastEvent(new ContextClosedEvent(this.context));
 		assertThat(bridgeHandlerInstalled()).isFalse();
 	}
 
@@ -396,7 +398,7 @@ public void shutdownHookIsNotRegisteredByDefault() throws Exception {
 		TestLoggingApplicationListener listener = new TestLoggingApplicationListener();
 		System.setProperty(LoggingSystem.class.getName(),
 				TestShutdownHandlerLoggingSystem.class.getName());
-		listener.onApplicationEvent(
+		multicastEvent(listener,
 				new ApplicationStartedEvent(new SpringApplication(), NO_ARGS));
 		listener.initialize(this.context.getEnvironment(), this.context.getClassLoader());
 		assertThat(listener.shutdownHook).isNull();
@@ -409,7 +411,7 @@ public void shutdownHookCanBeRegistered() throws Exception {
 				TestShutdownHandlerLoggingSystem.class.getName());
 		TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.context,
 				"logging.register_shutdown_hook=true");
-		listener.onApplicationEvent(
+		multicastEvent(listener,
 				new ApplicationStartedEvent(new SpringApplication(), NO_ARGS));
 		listener.initialize(this.context.getEnvironment(), this.context.getClassLoader());
 		assertThat(listener.shutdownHook).isNotNull();
@@ -422,29 +424,29 @@ public void shutdownHookCanBeRegistered() throws Exception {
 	public void closingContextCleansUpLoggingSystem() {
 		System.setProperty(LoggingSystem.SYSTEM_PROPERTY,
 				TestCleanupLoggingSystem.class.getName());
-		this.initializer.onApplicationEvent(
+		multicastEvent(
 				new ApplicationStartedEvent(this.springApplication, new String[0]));
 		TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils
 				.getField(this.initializer, "loggingSystem");
 		assertThat(loggingSystem.cleanedUp).isFalse();
-		this.initializer.onApplicationEvent(new ContextClosedEvent(this.context));
+		multicastEvent(new ContextClosedEvent(this.context));
 		assertThat(loggingSystem.cleanedUp).isTrue();
 	}
 
 	@Test
 	public void closingChildContextDoesNotCleanUpLoggingSystem() {
 		System.setProperty(LoggingSystem.SYSTEM_PROPERTY,
 				TestCleanupLoggingSystem.class.getName());
-		this.initializer.onApplicationEvent(
+		multicastEvent(
 				new ApplicationStartedEvent(this.springApplication, new String[0]));
 		TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils
 				.getField(this.initializer, "loggingSystem");
 		assertThat(loggingSystem.cleanedUp).isFalse();
 		GenericApplicationContext childContext = new GenericApplicationContext();
 		childContext.setParent(this.context);
-		this.initializer.onApplicationEvent(new ContextClosedEvent(childContext));
+		multicastEvent(new ContextClosedEvent(childContext));
 		assertThat(loggingSystem.cleanedUp).isFalse();
-		this.initializer.onApplicationEvent(new ContextClosedEvent(this.context));
+		multicastEvent(new ContextClosedEvent(this.context));
 		assertThat(loggingSystem.cleanedUp).isTrue();
 		childContext.close();
 	}
@@ -491,17 +493,26 @@ public void logFilePropertiesCanReferenceSystemProperties() {
 	public void applicationFailedEventCleansUpLoggingSystem() {
 		System.setProperty(LoggingSystem.SYSTEM_PROPERTY,
 				TestCleanupLoggingSystem.class.getName());
-		this.initializer.onApplicationEvent(
+		multicastEvent(
 				new ApplicationStartedEvent(this.springApplication, new String[0]));
 		TestCleanupLoggingSystem loggingSystem = (TestCleanupLoggingSystem) ReflectionTestUtils
 				.getField(this.initializer, "loggingSystem");
 		assertThat(loggingSystem.cleanedUp).isFalse();
-		this.initializer
-				.onApplicationEvent(new ApplicationFailedEvent(this.springApplication,
-						new String[0], new GenericApplicationContext(), new Exception()));
+		multicastEvent(new ApplicationFailedEvent(this.springApplication, new String[0],
+				new GenericApplicationContext(), new Exception()));
 		assertThat(loggingSystem.cleanedUp).isTrue();
 	}
 
+	private void multicastEvent(ApplicationEvent event) {
+		multicastEvent(this.initializer, event);
+	}
+
+	private void multicastEvent(ApplicationListener<?> listener, ApplicationEvent event) {
+		SimpleApplicationEventMulticaster multicaster = new SimpleApplicationEventMulticaster();
+		multicaster.addApplicationListener(listener);
+		multicaster.multicastEvent(event);
+	}
+
 	private boolean bridgeHandlerInstalled() {
 		Logger rootLogger = LogManager.getLogManager().getLogger("");
 		Handler[] handlers = rootLogger.getHandlers();