From c187bd928a407c9fe9025953e98946a3c1baa2f4 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 14 Sep 2023 15:23:24 +0100 Subject: [PATCH] Don't add runtime shutdown hook till app with hook enabled is run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the runtime shutdown hook was added as soon as a shutdown handler was registered. This causes a memory leak in a war deployment when the application is undeployed as LoggingApplicationListener always registers a shutdown handler so the runtime shutdown hook was always registered. This commit updates the shutdown hook so that the runtime shutdown hook is only allowed to be added once run() has been called on a SpringApplication with the shutdown hook enabled. This approach allows the registerShutdownHook flag on SpringApplication to be a central point of control for the registration of the runtime shutdown hook. When that flag is set to false, for example by SpringBootServletInitializer, the runtime shutdown hook will not be registered, irrespective of whether other code uses the public API to add a shutdown handler. An alternative approach of stopping LoggingApplicationListener from adding its shutdown handler – for example by adding logging.register-shutdown-hook=false to the environment – was considered. This approach was rejected in favor of the centralized approach described above as it would require every caller that adds a shutdown handler to deal with the problem. Closes gh-37096 --- .../springframework/boot/SpringApplication.java | 3 +++ .../boot/SpringApplicationShutdownHook.java | 8 +++++++- .../boot/SpringApplicationShutdownHookTests.java | 14 ++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java index 0531c28bab..69f42ed0ea 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java @@ -296,6 +296,9 @@ public class SpringApplication { * @return a running {@link ApplicationContext} */ public ConfigurableApplicationContext run(String... args) { + if (this.registerShutdownHook) { + SpringApplication.shutdownHook.enableShutdowHookAddition(); + } long startTime = System.nanoTime(); DefaultBootstrapContext bootstrapContext = createBootstrapContext(); ConfigurableApplicationContext context = null; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationShutdownHook.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationShutdownHook.java index afa1ea89e9..da985828df 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationShutdownHook.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationShutdownHook.java @@ -62,12 +62,18 @@ class SpringApplicationShutdownHook implements Runnable { private final AtomicBoolean shutdownHookAdded = new AtomicBoolean(); + private volatile boolean shutdownHookAdditionEnabled = false; + private boolean inProgress; SpringApplicationShutdownHandlers getHandlers() { return this.handlers; } + void enableShutdowHookAddition() { + this.shutdownHookAdditionEnabled = true; + } + void registerApplicationContext(ConfigurableApplicationContext context) { addRuntimeShutdownHookIfNecessary(); synchronized (SpringApplicationShutdownHook.class) { @@ -78,7 +84,7 @@ class SpringApplicationShutdownHook implements Runnable { } private void addRuntimeShutdownHookIfNecessary() { - if (this.shutdownHookAdded.compareAndSet(false, true)) { + if (this.shutdownHookAdditionEnabled && this.shutdownHookAdded.compareAndSet(false, true)) { addRuntimeShutdownHook(); } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationShutdownHookTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationShutdownHookTests.java index c8cf5b1ffb..0ddb5b8c88 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationShutdownHookTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationShutdownHookTests.java @@ -51,6 +51,7 @@ class SpringApplicationShutdownHookTests { @Test void shutdownHookIsNotAddedUntilContextIsRegistered() { TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook(); + shutdownHook.enableShutdowHookAddition(); assertThat(shutdownHook.isRuntimeShutdownHookAdded()).isFalse(); ConfigurableApplicationContext context = new GenericApplicationContext(); shutdownHook.registerApplicationContext(context); @@ -60,12 +61,25 @@ class SpringApplicationShutdownHookTests { @Test void shutdownHookIsNotAddedUntilHandlerIsRegistered() { TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook(); + shutdownHook.enableShutdowHookAddition(); assertThat(shutdownHook.isRuntimeShutdownHookAdded()).isFalse(); shutdownHook.getHandlers().add(() -> { }); assertThat(shutdownHook.isRuntimeShutdownHookAdded()).isTrue(); } + @Test + void shutdownHookIsNotAddedUntilAdditionIsEnabled() { + TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook(); + shutdownHook.getHandlers().add(() -> { + }); + assertThat(shutdownHook.isRuntimeShutdownHookAdded()).isFalse(); + shutdownHook.enableShutdowHookAddition(); + shutdownHook.getHandlers().add(() -> { + }); + assertThat(shutdownHook.isRuntimeShutdownHookAdded()).isTrue(); + } + @Test void runClosesContextsBeforeRunningHandlerActions() { TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook();