Skip to content

Commit

Permalink
Only close context that is active
Browse files Browse the repository at this point in the history
Previously, SpringApplicationShutdownHook would call close() on any
registered application context even if it wasn't active as it had
already been closed. This could lead to deadlock if the context was
closed and System.exit was called during application context refresh.

This commit updates SpringApplicationShutdownHook so that it only
calls close() on active contexts. This prevents deadlock as it avoids
trying to sychronize on the context's startupShutdownMonitor on
the shutdown hook thread while it's still held on the main thread
which called System.exit and is waiting for all of the shutdown hooks
to complete.

Fixes gh-27049
  • Loading branch information
wilkinsona committed Jun 24, 2021
1 parent 05b041b commit 5a9fa3c
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 0 deletions.
Expand Up @@ -125,6 +125,9 @@ void reset() {
* @param context the context to clean
*/
private void closeAndWait(ConfigurableApplicationContext context) {
if (!context.isActive()) {
return;
}
context.close();
try {
int waited = 0;
Expand Down
Expand Up @@ -26,10 +26,12 @@
import org.awaitility.Awaitility;
import org.junit.jupiter.api.Test;

import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.context.support.AbstractApplicationContext;
import org.springframework.context.support.GenericApplicationContext;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
Expand Down Expand Up @@ -91,6 +93,15 @@ void runWhenContextIsBeingClosedInAnotherThreadWaitsUntilContextIsInactive() thr
assertThat(finished).containsExactly(context, handlerAction);
}

@Test
void runDueToExitDuringRefreshWhenContextHasBeenClosedDoesNotDeadlock() throws InterruptedException {
GenericApplicationContext context = new GenericApplicationContext();
TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook();
shutdownHook.registerApplicationContext(context);
context.registerBean(CloseContextAndExit.class, context, shutdownHook);
context.refresh();
}

@Test
void runWhenContextIsClosedDirectlyRunsHandlerActions() {
TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook();
Expand Down Expand Up @@ -221,4 +232,28 @@ public void run() {

}

static class CloseContextAndExit implements InitializingBean {

private final ConfigurableApplicationContext context;

private final Runnable shutdownHook;

CloseContextAndExit(ConfigurableApplicationContext context, SpringApplicationShutdownHook shutdownHook) {
this.context = context;
this.shutdownHook = shutdownHook;
}

@Override
public void afterPropertiesSet() throws Exception {
this.context.close();
// Simulate System.exit by running the hook on a separate thread and waiting
// for it to complete
Thread thread = new Thread(this.shutdownHook);
thread.start();
thread.join(15000);
assertThat(thread.isAlive()).isFalse();
}

}

}

0 comments on commit 5a9fa3c

Please sign in to comment.