From 5a9fa3c8f1d40095c5521e107854d2a5dc0ce2df Mon Sep 17 00:00:00 2001
From: Andy Wilkinson <wilkinsona@vmware.com>
Date: Thu, 24 Jun 2021 12:47:42 +0100
Subject: [PATCH] Only close context that is active

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
---
 .../boot/SpringApplicationShutdownHook.java   |  3 ++
 .../SpringApplicationShutdownHookTests.java   | 35 +++++++++++++++++++
 2 files changed, 38 insertions(+)

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 c4bd7222194..cc4faff521e 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
@@ -125,6 +125,9 @@ class SpringApplicationShutdownHook implements Runnable {
 	 * @param context the context to clean
 	 */
 	private void closeAndWait(ConfigurableApplicationContext context) {
+		if (!context.isActive()) {
+			return;
+		}
 		context.close();
 		try {
 			int waited = 0;
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 12c7f9d5c52..d188a1b731e 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
@@ -26,10 +26,12 @@ import java.util.concurrent.CountDownLatch;
 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;
@@ -91,6 +93,15 @@ class SpringApplicationShutdownHookTests {
 		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();
@@ -221,4 +232,28 @@ class SpringApplicationShutdownHookTests {
 
 	}
 
+	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();
+		}
+
+	}
+
 }