From 180815445c5271dbee42d1983e7724d95b2b38a6 Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Tue, 27 Oct 2015 13:47:59 +0900 Subject: [PATCH 1/2] Fix registration of shutdown handler as a shutdown hook Closes gh-4311 --- .../boot/logging/LoggingApplicationListener.java | 2 +- 1 file changed, 1 insertion(+), 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 d2e7760f65..5bee1bab74 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 @@ -337,7 +337,7 @@ public class LoggingApplicationListener implements GenericApplicationListener { Runnable shutdownHandler = loggingSystem.getShutdownHandler(); if (shutdownHandler != null && shutdownHookRegistered.compareAndSet(false, true)) { - Runtime.getRuntime().addShutdownHook(new Thread()); + Runtime.getRuntime().addShutdownHook(new Thread(shutdownHandler)); } } } From 56334b48f6c22af5f981d17efc5fabaa3d636d7c Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 27 Oct 2015 09:54:43 +0000 Subject: [PATCH 2/2] Improve testing of shutdown handler registration --- .../logging/LoggingApplicationListener.java | 6 +- .../LoggingApplicationListenerTests.java | 55 ++++++++++++++----- 2 files changed, 45 insertions(+), 16 deletions(-) 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 5bee1bab74..fe5fd89bd4 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 @@ -337,11 +337,15 @@ public class LoggingApplicationListener implements GenericApplicationListener { Runnable shutdownHandler = loggingSystem.getShutdownHandler(); if (shutdownHandler != null && shutdownHookRegistered.compareAndSet(false, true)) { - Runtime.getRuntime().addShutdownHook(new Thread(shutdownHandler)); + registerShutdownHook(new Thread(shutdownHandler)); } } } + void registerShutdownHook(Thread shutdownHook) { + Runtime.getRuntime().addShutdownHook(shutdownHook); + } + public void setOrder(int order) { this.order = order; } 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 cb17cc0754..186c10ecd8 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 @@ -18,6 +18,8 @@ package org.springframework.boot.logging; import java.io.File; import java.io.IOException; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.logging.Handler; import java.util.logging.LogManager; import java.util.logging.Logger; @@ -44,6 +46,7 @@ import org.springframework.context.support.GenericApplicationContext; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -346,26 +349,29 @@ public class LoggingApplicationListenerTests { @Test public void shutdownHookIsNotRegisteredByDefault() throws Exception { + TestLoggingApplicationListener listener = new TestLoggingApplicationListener(); System.setProperty(LoggingSystem.class.getName(), - NullShutdownHandlerLoggingSystem.class.getName()); - this.initializer.onApplicationEvent( + TestShutdownHandlerLoggingSystem.class.getName()); + listener.onApplicationEvent( new ApplicationStartedEvent(new SpringApplication(), NO_ARGS)); - this.initializer.initialize(this.context.getEnvironment(), - this.context.getClassLoader()); - assertThat(NullShutdownHandlerLoggingSystem.shutdownHandlerRequested, is(false)); + listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + assertThat(listener.shutdownHook, is(nullValue())); } @Test public void shutdownHookCanBeRegistered() throws Exception { + TestLoggingApplicationListener listener = new TestLoggingApplicationListener(); System.setProperty(LoggingSystem.class.getName(), - NullShutdownHandlerLoggingSystem.class.getName()); + TestShutdownHandlerLoggingSystem.class.getName()); EnvironmentTestUtils.addEnvironment(this.context, "logging.register_shutdown_hook:true"); - this.initializer.onApplicationEvent( + listener.onApplicationEvent( new ApplicationStartedEvent(new SpringApplication(), NO_ARGS)); - this.initializer.initialize(this.context.getEnvironment(), - this.context.getClassLoader()); - assertThat(NullShutdownHandlerLoggingSystem.shutdownHandlerRequested, is(true)); + listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); + assertThat(listener.shutdownHook, is(not(nullValue()))); + listener.shutdownHook.start(); + assertThat(TestShutdownHandlerLoggingSystem.shutdownLatch.await(30, + TimeUnit.SECONDS), is(true)); } private boolean bridgeHandlerInstalled() { @@ -379,12 +385,13 @@ public class LoggingApplicationListenerTests { return false; } - public static class NullShutdownHandlerLoggingSystem extends AbstractLoggingSystem { + public static class TestShutdownHandlerLoggingSystem extends AbstractLoggingSystem { - static boolean shutdownHandlerRequested = false; + private static CountDownLatch shutdownLatch; - public NullShutdownHandlerLoggingSystem(ClassLoader classLoader) { + public TestShutdownHandlerLoggingSystem(ClassLoader classLoader) { super(classLoader); + TestShutdownHandlerLoggingSystem.shutdownLatch = new CountDownLatch(1); } @Override @@ -410,8 +417,26 @@ public class LoggingApplicationListenerTests { @Override public Runnable getShutdownHandler() { - shutdownHandlerRequested = true; - return null; + return new Runnable() { + + @Override + public void run() { + TestShutdownHandlerLoggingSystem.shutdownLatch.countDown(); + } + + }; + } + + } + + public static class TestLoggingApplicationListener + extends LoggingApplicationListener { + + private Thread shutdownHook; + + @Override + void registerShutdownHook(Thread shutdownHook) { + this.shutdownHook = shutdownHook; } }