Skip to content

Commit

Permalink
Don't add runtime shutdown hook till app with hook enabled is run
Browse files Browse the repository at this point in the history
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
  • Loading branch information
wilkinsona committed Sep 15, 2023
1 parent d9207fc commit c187bd9
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,9 @@ private Optional<Class<?>> findMainClass(Stream<StackFrame> stack) {
* @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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -78,7 +84,7 @@ void registerApplicationContext(ConfigurableApplicationContext context) {
}

private void addRuntimeShutdownHookIfNecessary() {
if (this.shutdownHookAdded.compareAndSet(false, true)) {
if (this.shutdownHookAdditionEnabled && this.shutdownHookAdded.compareAndSet(false, true)) {
addRuntimeShutdownHook();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -60,12 +61,25 @@ void shutdownHookIsNotAddedUntilContextIsRegistered() {
@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();
Expand Down

0 comments on commit c187bd9

Please sign in to comment.