From aaae4aa3a17f068b030b2cb05d8e3a6e134c3cbd Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 29 Oct 2015 12:37:04 +0000 Subject: [PATCH] Prevent restarts from switching off Log4J2-based logging During a restart, the Restarter runs all registered shutdown hooks. This breaks Log4J2 as it leaves it in a shutdown state that leaves logging switched off such that no output it produced when the application starts up again. This commit introduces a new RestartListener abstraction. RestartListeners are notified prior to the application being restarted. A Log4J2-specific implementation is provided that prepares Log4J2 for restart by removing any shutdown callbacks from its shutdown callback registry. This prevents the restart from shutting down Log4J2, ensuring that it still functions when the application restarts. Closes gh-4279 --- spring-boot-devtools/pom.xml | 15 +++-- .../log4j2/Log4J2RestartListener.java | 62 +++++++++++++++++++ .../restart/RestartApplicationListener.java | 9 ++- .../devtools/restart/RestartListener.java | 31 ++++++++++ .../boot/devtools/restart/Restarter.java | 19 +++++- .../main/resources/META-INF/spring.factories | 4 ++ .../boot/devtools/restart/RestarterTests.java | 15 ++++- 7 files changed, 145 insertions(+), 10 deletions(-) create mode 100644 spring-boot-devtools/src/main/java/org/springframework/boot/devtools/log4j2/Log4J2RestartListener.java create mode 100644 spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/RestartListener.java diff --git a/spring-boot-devtools/pom.xml b/spring-boot-devtools/pom.xml index b5641034ab..e5dc9709ec 100644 --- a/spring-boot-devtools/pom.xml +++ b/spring-boot-devtools/pom.xml @@ -30,6 +30,16 @@ spring-boot-autoconfigure + + javax.servlet + javax.servlet-api + true + + + org.apache.logging.log4j + log4j-core + true + org.springframework spring-web @@ -45,11 +55,6 @@ spring-security-web true - - javax.servlet - javax.servlet-api - true - org.springframework.boot diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/log4j2/Log4J2RestartListener.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/log4j2/Log4J2RestartListener.java new file mode 100644 index 0000000000..bb62e51f5b --- /dev/null +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/log4j2/Log4J2RestartListener.java @@ -0,0 +1,62 @@ +/* + * Copyright 2012-2015 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.devtools.log4j2; + +import java.lang.reflect.Field; +import java.util.Collection; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.util.Cancellable; +import org.apache.logging.log4j.core.util.ShutdownCallbackRegistry; +import org.apache.logging.log4j.spi.LoggerContextFactory; + +import org.springframework.boot.devtools.restart.RestartListener; +import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; + +/** + * {@link RestartListener} that prepares Log4J2 for an application restart. + * + * @author Andy Wilkinson + */ +public class Log4J2RestartListener implements RestartListener { + + @Override + public void beforeRestart() { + if (ClassUtils.isPresent("org.apache.logging.log4j.LogManager", + getClass().getClassLoader())) { + prepareLog4J2ForRestart(); + } + } + + private void prepareLog4J2ForRestart() { + LoggerContextFactory factory = LogManager.getFactory(); + Field field = ReflectionUtils.findField(factory.getClass(), + "shutdownCallbackRegistry"); + ReflectionUtils.makeAccessible(field); + ShutdownCallbackRegistry shutdownCallbackRegistry = (ShutdownCallbackRegistry) ReflectionUtils + .getField(field, factory); + Field hooksField = ReflectionUtils.findField(shutdownCallbackRegistry.getClass(), + "hooks"); + ReflectionUtils.makeAccessible(hooksField); + @SuppressWarnings("unchecked") + Collection state = (Collection) ReflectionUtils + .getField(hooksField, shutdownCallbackRegistry); + state.clear(); + } + +} diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/RestartApplicationListener.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/RestartApplicationListener.java index 0b8e718bea..db31b1d846 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/RestartApplicationListener.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/RestartApplicationListener.java @@ -16,12 +16,15 @@ package org.springframework.boot.devtools.restart; +import java.util.List; + import org.springframework.boot.context.event.ApplicationFailedEvent; import org.springframework.boot.context.event.ApplicationReadyEvent; import org.springframework.boot.context.event.ApplicationStartedEvent; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationListener; import org.springframework.core.Ordered; +import org.springframework.core.io.support.SpringFactoriesLoader; /** * {@link ApplicationListener} to initialize the {@link Restarter}. @@ -56,7 +59,11 @@ public class RestartApplicationListener String[] args = event.getArgs(); DefaultRestartInitializer initializer = new DefaultRestartInitializer(); boolean restartOnInitialize = !AgentReloader.isActive(); - Restarter.initialize(args, false, initializer, restartOnInitialize); + List restartListeners = SpringFactoriesLoader + .loadFactories(RestartListener.class, getClass().getClassLoader()); + Restarter.initialize(args, false, initializer, restartOnInitialize, + restartListeners + .toArray(new RestartListener[restartListeners.size()])); } else { Restarter.disable(); diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/RestartListener.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/RestartListener.java new file mode 100644 index 0000000000..fe30e6be2b --- /dev/null +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/RestartListener.java @@ -0,0 +1,31 @@ +/* + * Copyright 2012-2015 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.devtools.restart; + +/** + * Listener that is notified of application restarts. + * + * @author Andy Wilkinson + */ +public interface RestartListener { + + /** + * Called before an application restart. + */ + void beforeRestart(); + +} diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java index aec0340451..c08175c796 100644 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/restart/Restarter.java @@ -108,6 +108,8 @@ public class Restarter { private final BlockingDeque leakSafeThreads = new LinkedBlockingDeque(); + private final RestartListener[] listeners; + private boolean finished = false; private Lock stopLock = new ReentrantLock(); @@ -118,10 +120,11 @@ public class Restarter { * @param args the application arguments * @param forceReferenceCleanup if soft/weak reference cleanup should be forced * @param initializer the restart initializer + * @param listeners listeners to be notified of restarts * @see #initialize(String[]) */ protected Restarter(Thread thread, String[] args, boolean forceReferenceCleanup, - RestartInitializer initializer) { + RestartInitializer initializer, RestartListener... listeners) { Assert.notNull(thread, "Thread must not be null"); Assert.notNull(args, "Args must not be null"); Assert.notNull(initializer, "Initializer must not be null"); @@ -134,6 +137,7 @@ public class Restarter { this.args = args; this.exceptionHandler = thread.getUncaughtExceptionHandler(); this.leakSafeThreads.add(new LeakSafeThread()); + this.listeners = listeners; } private String getMainClassName(Thread thread) { @@ -246,6 +250,7 @@ public class Restarter { @Override public Void call() throws Exception { + Restarter.this.beforeRestart(); Restarter.this.stop(); Restarter.this.start(failureHandler); return null; @@ -324,6 +329,12 @@ public class Restarter { System.runFinalization(); } + private void beforeRestart() { + for (RestartListener listener : this.listeners) { + listener.beforeRestart(); + } + } + @SuppressWarnings("rawtypes") private void triggerShutdownHooks() throws Exception { Class hooksClass = Class.forName("java.lang.ApplicationShutdownHooks"); @@ -512,13 +523,15 @@ public class Restarter { * @param initializer the restart initializer * @param restartOnInitialize if the restarter should be restarted immediately when * the {@link RestartInitializer} returns non {@code null} results + * @param listeners listeners to be notified of restarts */ public static void initialize(String[] args, boolean forceReferenceCleanup, - RestartInitializer initializer, boolean restartOnInitialize) { + RestartInitializer initializer, boolean restartOnInitialize, + RestartListener... listeners) { if (instance == null) { synchronized (Restarter.class) { instance = new Restarter(Thread.currentThread(), args, - forceReferenceCleanup, initializer); + forceReferenceCleanup, initializer, listeners); } instance.initialize(restartOnInitialize); } diff --git a/spring-boot-devtools/src/main/resources/META-INF/spring.factories b/spring-boot-devtools/src/main/resources/META-INF/spring.factories index c5923c0b13..fc66a3dcdb 100644 --- a/spring-boot-devtools/src/main/resources/META-INF/spring.factories +++ b/spring-boot-devtools/src/main/resources/META-INF/spring.factories @@ -15,3 +15,7 @@ org.springframework.boot.devtools.autoconfigure.RemoteDevToolsAutoConfiguration org.springframework.boot.env.EnvironmentPostProcessor=\ org.springframework.boot.devtools.env.DevToolsHomePropertiesPostProcessor,\ org.springframework.boot.devtools.env.DevToolsPropertyDefaultsPostProcessor + +# Restart Listeners +org.springframework.boot.devtools.restart.RestartListener=\ +org.springframework.boot.devtools.log4j2.Log4J2RestartListener diff --git a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/restart/RestarterTests.java b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/restart/RestarterTests.java index aa033a40f6..f05342ec26 100644 --- a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/restart/RestarterTests.java +++ b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/restart/RestarterTests.java @@ -96,6 +96,7 @@ public class RestarterTests { String output = this.out.toString(); assertThat(StringUtils.countOccurrencesOf(output, "Tick 0"), greaterThan(1)); assertThat(StringUtils.countOccurrencesOf(output, "Tick 1"), greaterThan(1)); + assertThat(TestRestartListener.restarts, greaterThan(1)); } @Test @@ -214,7 +215,8 @@ public class RestarterTests { } public static void main(String... args) { - Restarter.initialize(args, false, new MockRestartInitializer()); + Restarter.initialize(args, false, new MockRestartInitializer(), true, + new TestRestartListener()); AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( SampleApplication.class); context.registerShutdownHook(); @@ -276,4 +278,15 @@ public class RestarterTests { } + private static class TestRestartListener implements RestartListener { + + private static int restarts; + + @Override + public void beforeRestart() { + restarts++; + } + + } + }