From f3f119b111d21785bca0772526aeb18a2e57f2f9 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 9 Jun 2021 17:00:48 +0100 Subject: [PATCH] Don't shutdown logging system before contexts Add `SpringApplicationShutdownHook` to manage orderly application shutdown, specifically around the `LoggingSystem`. `SpringApplication` now offers a `getShutdownHandlers()` method that can be used to add handlers that are guaranteed to only run after the `ApplicationContext` has been closed and is inactive. Fixes gh-26660 --- .../boot/SpringApplication.java | 21 +- .../SpringApplicationShutdownHandlers.java | 49 ++++ .../boot/SpringApplicationShutdownHook.java | 205 ++++++++++++++++ .../logging/LoggingApplicationListener.java | 14 +- ...SpringApplicationShutdownHookInstance.java | 85 +++++++ .../SpringApplicationShutdownHookTests.java | 224 ++++++++++++++++++ .../boot/SpringApplicationTests.java | 26 +- .../SpringApplicationBuilderTests.java | 22 +- .../LoggingApplicationListenerTests.java | 6 +- 9 files changed, 606 insertions(+), 46 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationShutdownHandlers.java create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationShutdownHook.java create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationShutdownHookInstance.java create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationShutdownHookTests.java 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 60a5d6dc76b6..8bed28879465 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 @@ -17,7 +17,6 @@ package org.springframework.boot; import java.lang.reflect.Constructor; -import java.security.AccessControlException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -201,6 +200,8 @@ public class SpringApplication { private static final Log logger = LogFactory.getLog(SpringApplication.class); + static final SpringApplicationShutdownHook shutdownHook = new SpringApplicationShutdownHook(); + private Set> primarySources; private Set sources = new LinkedHashSet<>(); @@ -428,12 +429,7 @@ private void prepareContext(DefaultBootstrapContext bootstrapContext, Configurab private void refreshContext(ConfigurableApplicationContext context) { if (this.registerShutdownHook) { - try { - context.registerShutdownHook(); - } - catch (AccessControlException ex) { - // Not allowed in some environments. - } + shutdownHook.registerApplicationContext(context); } refresh(context); } @@ -987,6 +983,7 @@ public void setHeadless(boolean headless) { * registered. Defaults to {@code true} to ensure that JVM shutdowns are handled * gracefully. * @param registerShutdownHook if the shutdown hook should be registered + * @see #getShutdownHandlers() */ public void setRegisterShutdownHook(boolean registerShutdownHook) { this.registerShutdownHook = registerShutdownHook; @@ -1314,6 +1311,16 @@ public ApplicationStartup getApplicationStartup() { return this.applicationStartup; } + /** + * Return a {@link SpringApplicationShutdownHandlers} instance that can be used to add + * or remove handlers that perform actions before the JVM is shutdown. + * @return a {@link SpringApplicationShutdownHandlers} instance + * @since 2.5.1 + */ + public static SpringApplicationShutdownHandlers getShutdownHandlers() { + return shutdownHook.getHandlers(); + } + /** * Static helper that can be used to run a {@link SpringApplication} from the * specified source using default settings. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationShutdownHandlers.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationShutdownHandlers.java new file mode 100644 index 000000000000..882d1debf1b8 --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationShutdownHandlers.java @@ -0,0 +1,49 @@ +/* + * Copyright 2012-2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot; + +import org.springframework.context.ApplicationContext; + +/** + * Interface that can be used to add or remove code that should run when the JVM is + * shutdown. Shutdown handers are similar to JVM {@link Runtime#addShutdownHook(Thread) + * shutdown hooks} except that they run sequentially rather than concurrently. + *

+ * Shutdown handlers are guaranteed to be called only after registered + * {@link ApplicationContext} instances have been closed and are no longer active. + * + * @author Phillip Webb + * @author Andy Wilkinson + * @since 2.5.1 + * @see SpringApplication#getShutdownHandlers() + * @see SpringApplication#setRegisterShutdownHook(boolean) + */ +public interface SpringApplicationShutdownHandlers { + + /** + * Add an action to the handlers that will be run when the JVM exits. + * @param action the action to add + */ + void add(Runnable action); + + /** + * Remove a previously added an action so that it no longer runs when the JVM exits. + * @param action the action to remove + */ + void remove(Runnable action); + +} 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 new file mode 100644 index 000000000000..c4bd7222194e --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/SpringApplicationShutdownHook.java @@ -0,0 +1,205 @@ +/* + * Copyright 2012-2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot; + +import java.security.AccessControlException; +import java.util.Collections; +import java.util.IdentityHashMap; +import java.util.LinkedHashSet; +import java.util.Set; +import java.util.WeakHashMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationListener; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.event.ContextClosedEvent; +import org.springframework.util.Assert; + +/** + * A {@link Runnable} to be used as a {@link Runtime#addShutdownHook(Thread) shutdown + * hook} to perform graceful shutdown of Spring Boot applications. This hook tracks + * registered application contexts as well as any actions registered via + * {@link SpringApplication#getShutdownHandlers()}. + * + * @author Andy Wilkinson + * @author Phillip Webb + */ +class SpringApplicationShutdownHook implements Runnable { + + private static final int SLEEP = 50; + + private static final long TIMEOUT = TimeUnit.MINUTES.toMillis(10); + + private static final Log logger = LogFactory.getLog(SpringApplicationShutdownHook.class); + + private final Handlers handlers = new Handlers(); + + private final Set contexts = new LinkedHashSet<>(); + + private final Set closedContexts = Collections.newSetFromMap(new WeakHashMap<>()); + + private final ApplicationContextClosedListener contextCloseListener = new ApplicationContextClosedListener(); + + private boolean inProgress; + + SpringApplicationShutdownHook() { + try { + addRuntimeShutdownHook(); + } + catch (AccessControlException ex) { + // Not allowed in some environments + } + } + + protected void addRuntimeShutdownHook() { + Runtime.getRuntime().addShutdownHook(new Thread(this, "SpringApplicationShutdownHook")); + } + + SpringApplicationShutdownHandlers getHandlers() { + return this.handlers; + } + + void registerApplicationContext(ConfigurableApplicationContext context) { + synchronized (SpringApplicationShutdownHook.class) { + assertNotInProgress(); + context.addApplicationListener(this.contextCloseListener); + this.contexts.add(context); + } + } + + @Override + public void run() { + Set contexts; + Set closedContexts; + Set actions; + synchronized (SpringApplicationShutdownHook.class) { + this.inProgress = true; + contexts = new LinkedHashSet<>(this.contexts); + closedContexts = new LinkedHashSet<>(this.closedContexts); + actions = new LinkedHashSet<>(this.handlers.getActions()); + } + contexts.forEach(this::closeAndWait); + closedContexts.forEach(this::closeAndWait); + actions.forEach(Runnable::run); + } + + boolean isApplicationContextRegistered(ConfigurableApplicationContext context) { + synchronized (SpringApplicationShutdownHook.class) { + return this.contexts.contains(context); + } + } + + void reset() { + synchronized (SpringApplicationShutdownHook.class) { + this.contexts.clear(); + this.closedContexts.clear(); + this.handlers.getActions().clear(); + this.inProgress = false; + } + } + + /** + * Call {@link ConfigurableApplicationContext#close()} and wait until the context + * becomes inactive. We can't assume that just because the close method returns that + * the context is actually inactive. It could be that another thread is still in the + * process of disposing beans. + * @param context the context to clean + */ + private void closeAndWait(ConfigurableApplicationContext context) { + context.close(); + try { + int waited = 0; + while (context.isActive()) { + if (waited > TIMEOUT) { + throw new TimeoutException(); + } + Thread.sleep(SLEEP); + waited += SLEEP; + } + } + catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + logger.warn("Interrupted waiting for application context " + context + " to become inactive"); + } + catch (TimeoutException ex) { + logger.warn("Timed out waiting for application context " + context + " to become inactive", ex); + } + } + + private void assertNotInProgress() { + Assert.state(!SpringApplicationShutdownHook.this.inProgress, "Shutdown in progress"); + } + + /** + * The handler actions for this shutdown hook. + */ + private class Handlers implements SpringApplicationShutdownHandlers { + + private final Set actions = Collections.newSetFromMap(new IdentityHashMap<>()); + + @Override + public void add(Runnable action) { + Assert.notNull(action, "Action must not be null"); + synchronized (SpringApplicationShutdownHook.class) { + assertNotInProgress(); + this.actions.add(action); + } + } + + @Override + public void remove(Runnable action) { + Assert.notNull(action, "Action must not be null"); + synchronized (SpringApplicationShutdownHook.class) { + assertNotInProgress(); + this.actions.remove(action); + } + } + + Set getActions() { + return this.actions; + } + + } + + /** + * {@link ApplicationListener} to track closed contexts. + */ + private class ApplicationContextClosedListener implements ApplicationListener { + + @Override + public void onApplicationEvent(ContextClosedEvent event) { + // The ContextClosedEvent is fired at the start of a call to {@code close()} + // and if that happens in a different thread then the context may still be + // active. Rather than just removing the context, we add it to a {@code + // closedContexts} set. This is weak set so that the context can be GC'd once + // the {@code close()} method returns. + synchronized (SpringApplicationShutdownHook.class) { + ApplicationContext applicationContext = event.getApplicationContext(); + SpringApplicationShutdownHook.this.contexts.remove(applicationContext); + SpringApplicationShutdownHook.this.closedContexts + .add((ConfigurableApplicationContext) applicationContext); + } + } + + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java index b411a814e2f7..a2bb0ffc3ac9 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java @@ -233,10 +233,11 @@ private void onApplicationStartingEvent(ApplicationStartingEvent event) { } private void onApplicationEnvironmentPreparedEvent(ApplicationEnvironmentPreparedEvent event) { + SpringApplication springApplication = event.getSpringApplication(); if (this.loggingSystem == null) { - this.loggingSystem = LoggingSystem.get(event.getSpringApplication().getClassLoader()); + this.loggingSystem = LoggingSystem.get(springApplication.getClassLoader()); } - initialize(event.getEnvironment(), event.getSpringApplication().getClassLoader()); + initialize(event.getEnvironment(), springApplication.getClassLoader()); } private void onApplicationPreparedEvent(ApplicationPreparedEvent event) { @@ -398,17 +399,16 @@ private BiConsumer getLogLevelConfigurer(LoggingSystem system) } private void registerShutdownHookIfNecessary(Environment environment, LoggingSystem loggingSystem) { - boolean registerShutdownHook = environment.getProperty(REGISTER_SHUTDOWN_HOOK_PROPERTY, Boolean.class, true); - if (registerShutdownHook) { + if (environment.getProperty(REGISTER_SHUTDOWN_HOOK_PROPERTY, Boolean.class, true)) { Runnable shutdownHandler = loggingSystem.getShutdownHandler(); if (shutdownHandler != null && shutdownHookRegistered.compareAndSet(false, true)) { - registerShutdownHook(new Thread(shutdownHandler)); + registerShutdownHook(shutdownHandler); } } } - void registerShutdownHook(Thread shutdownHook) { - Runtime.getRuntime().addShutdownHook(shutdownHook); + void registerShutdownHook(Runnable shutdownHandler) { + SpringApplication.getShutdownHandlers().add(shutdownHandler); } public void setOrder(int order) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationShutdownHookInstance.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationShutdownHookInstance.java new file mode 100644 index 000000000000..1c3f068ff1a6 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationShutdownHookInstance.java @@ -0,0 +1,85 @@ +/* + * Copyright 2012-2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot; + +import org.assertj.core.api.AbstractBooleanAssert; +import org.assertj.core.api.AssertProvider; +import org.assertj.core.api.Assertions; +import org.assertj.core.api.ObjectAssert; + +import org.springframework.boot.SpringApplicationShutdownHookInstance.Assert; +import org.springframework.context.ConfigurableApplicationContext; + +/** + * Test access to the static {@link SpringApplicationShutdownHook} instance in + * {@link SpringApplication}. + * + * @author Phillip Webb + */ +public final class SpringApplicationShutdownHookInstance implements AssertProvider { + + private final SpringApplicationShutdownHook shutdownHook; + + private SpringApplicationShutdownHookInstance(SpringApplicationShutdownHook shutdownHook) { + this.shutdownHook = shutdownHook; + } + + SpringApplicationShutdownHook getShutdownHook() { + return this.shutdownHook; + } + + @Override + public Assert assertThat() { + return new Assert(this.shutdownHook); + } + + public static void reset() { + get().getShutdownHook().reset(); + } + + public static SpringApplicationShutdownHookInstance get() { + return new SpringApplicationShutdownHookInstance(SpringApplication.shutdownHook); + } + + /** + * Assertions that can be performed on the {@link SpringApplicationShutdownHook}. + */ + public static class Assert extends ObjectAssert { + + Assert(SpringApplicationShutdownHook actual) { + super(actual); + } + + public Assert registeredApplicationContext(ConfigurableApplicationContext context) { + assertThatIsApplicationContextRegistered(context).isTrue(); + return this; + } + + public Assert didNotRegisterApplicationContext(ConfigurableApplicationContext context) { + assertThatIsApplicationContextRegistered(context).isFalse(); + return this; + } + + private AbstractBooleanAssert assertThatIsApplicationContextRegistered( + ConfigurableApplicationContext context) { + return Assertions.assertThat(this.actual.isApplicationContextRegistered(context)) + .as("ApplicationContext registered with shutdown hook"); + } + + } + +} 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 new file mode 100644 index 000000000000..12c7f9d5c52e --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationShutdownHookTests.java @@ -0,0 +1,224 @@ +/* + * Copyright 2012-2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot; + +import java.lang.Thread.State; +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; + +import org.awaitility.Awaitility; +import org.junit.jupiter.api.Test; + +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 static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; + +/** + * Tests for {@link SpringApplicationShutdownHook}. + * + * @author Phillip Webb + * @author Andy Wilkinson + */ +class SpringApplicationShutdownHookTests { + + @Test + void createCallsRegister() { + TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook(); + assertThat(shutdownHook.isRuntimeShutdownHookAdded()).isTrue(); + } + + @Test + void runClosesContextsBeforeRunningHandlerActions() { + TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook(); + List finished = new CopyOnWriteArrayList<>(); + ConfigurableApplicationContext context = new TestApplicationContext(finished); + shutdownHook.registerApplicationContext(context); + context.refresh(); + Runnable handlerAction = new TestHandlerAction(finished); + shutdownHook.getHandlers().add(handlerAction); + shutdownHook.run(); + assertThat(finished).containsExactly(context, handlerAction); + } + + @Test + void runWhenContextIsBeingClosedInAnotherThreadWaitsUntilContextIsInactive() throws InterruptedException { + // This situation occurs in the Spring Tools IDE. It triggers a context close via + // JMX and then stops the JVM. The two actions happen almost simultaneously + TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook(); + List finished = new CopyOnWriteArrayList<>(); + CountDownLatch closing = new CountDownLatch(1); + CountDownLatch proceedWithClose = new CountDownLatch(1); + ConfigurableApplicationContext context = new TestApplicationContext(finished, closing, proceedWithClose); + shutdownHook.registerApplicationContext(context); + context.refresh(); + Runnable handlerAction = new TestHandlerAction(finished); + shutdownHook.getHandlers().add(handlerAction); + Thread contextThread = new Thread(context::close); + contextThread.start(); + // Wait for context thread to begin closing the context + closing.await(); + Thread shutdownThread = new Thread(shutdownHook); + shutdownThread.start(); + // Shutdown thread should become blocked on monitor held by context thread + Awaitility.await().atMost(Duration.ofSeconds(30)).until(shutdownThread::getState, State.BLOCKED::equals); + // Allow context thread to proceed, unblocking shutdown thread + proceedWithClose.countDown(); + contextThread.join(); + shutdownThread.join(); + // Context should have been closed before handler action was run + assertThat(finished).containsExactly(context, handlerAction); + } + + @Test + void runWhenContextIsClosedDirectlyRunsHandlerActions() { + TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook(); + List finished = new CopyOnWriteArrayList<>(); + ConfigurableApplicationContext context = new TestApplicationContext(finished); + shutdownHook.registerApplicationContext(context); + context.refresh(); + context.close(); + Runnable handlerAction1 = new TestHandlerAction(finished); + Runnable handlerAction2 = new TestHandlerAction(finished); + shutdownHook.getHandlers().add(handlerAction1); + shutdownHook.getHandlers().add(handlerAction2); + shutdownHook.run(); + assertThat(finished).contains(handlerAction1, handlerAction2); + } + + @Test + void addHandlerActionWhenNullThrowsException() { + TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook(); + assertThatIllegalArgumentException().isThrownBy(() -> shutdownHook.getHandlers().add(null)) + .withMessage("Action must not be null"); + } + + @Test + void addHandlerActionWhenShuttingDownThrowsException() { + TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook(); + shutdownHook.run(); + Runnable handlerAction = new TestHandlerAction(new ArrayList<>()); + assertThatIllegalStateException().isThrownBy(() -> shutdownHook.getHandlers().add(handlerAction)) + .withMessage("Shutdown in progress"); + } + + @Test + void removeHandlerActionWhenNullThrowsException() { + TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook(); + assertThatIllegalArgumentException().isThrownBy(() -> shutdownHook.getHandlers().remove(null)) + .withMessage("Action must not be null"); + } + + @Test + void removeHandlerActionWhenShuttingDownThrowsException() { + TestSpringApplicationShutdownHook shutdownHook = new TestSpringApplicationShutdownHook(); + Runnable handlerAction = new TestHandlerAction(new ArrayList<>()); + shutdownHook.getHandlers().add(handlerAction); + shutdownHook.run(); + assertThatIllegalStateException().isThrownBy(() -> shutdownHook.getHandlers().remove(handlerAction)) + .withMessage("Shutdown in progress"); + } + + static class TestSpringApplicationShutdownHook extends SpringApplicationShutdownHook { + + private boolean runtimeShutdownHookAdded; + + @Override + protected void addRuntimeShutdownHook() { + this.runtimeShutdownHookAdded = true; + } + + boolean isRuntimeShutdownHookAdded() { + return this.runtimeShutdownHookAdded; + } + + } + + static class TestApplicationContext extends AbstractApplicationContext { + + private final ConfigurableListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + + private final List finished; + + private final CountDownLatch closing; + + private final CountDownLatch proceedWithClose; + + TestApplicationContext(List finished) { + this(finished, null, null); + } + + TestApplicationContext(List finished, CountDownLatch closing, CountDownLatch proceedWithClose) { + this.finished = finished; + this.closing = closing; + this.proceedWithClose = proceedWithClose; + } + + @Override + protected void refreshBeanFactory() { + } + + @Override + protected void closeBeanFactory() { + } + + @Override + protected void onClose() { + if (this.closing != null) { + this.closing.countDown(); + } + if (this.proceedWithClose != null) { + try { + this.proceedWithClose.await(); + } + catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + } + } + this.finished.add(this); + } + + @Override + public ConfigurableListableBeanFactory getBeanFactory() { + return this.beanFactory; + } + + } + + static class TestHandlerAction implements Runnable { + + private final List finished; + + TestHandlerAction(List finished) { + this.finished = finished; + } + + @Override + public void run() { + this.finished.add(this); + } + + } + +} diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java index f9c0de4dd646..7d88270f19d6 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/SpringApplicationTests.java @@ -184,6 +184,7 @@ void cleanUp() { } System.clearProperty("spring.main.banner-mode"); System.clearProperty(CachedIntrospectionResults.IGNORE_BEANINFO_PROPERTY_NAME); + SpringApplicationShutdownHookInstance.reset(); } @Test @@ -910,10 +911,18 @@ void commandLineArgsApplyToSpringApplication() { @Test void registerShutdownHook() { SpringApplication application = new SpringApplication(ExampleConfig.class); - application.setApplicationContextFactory(ApplicationContextFactory.ofContextClass(SpyApplicationContext.class)); + application.setWebApplicationType(WebApplicationType.NONE); + this.context = application.run(); + assertThat(SpringApplicationShutdownHookInstance.get()).registeredApplicationContext(this.context); + } + + @Test + void registerShutdownHookOff() { + SpringApplication application = new SpringApplication(ExampleConfig.class); + application.setWebApplicationType(WebApplicationType.NONE); + application.setRegisterShutdownHook(false); this.context = application.run(); - SpyApplicationContext applicationContext = (SpyApplicationContext) this.context; - verify(applicationContext.getApplicationContext()).registerShutdownHook(); + assertThat(SpringApplicationShutdownHookInstance.get()).didNotRegisterApplicationContext(this.context); } @Test @@ -1009,16 +1018,6 @@ void applicationListenerFromContextIsCalledWhenContextFailsRefreshAfterListenerR verifyNoMoreInteractions(listener); } - @Test - void registerShutdownHookOff() { - SpringApplication application = new SpringApplication(ExampleConfig.class); - application.setApplicationContextFactory(ApplicationContextFactory.ofContextClass(SpyApplicationContext.class)); - application.setRegisterShutdownHook(false); - this.context = application.run(); - SpyApplicationContext applicationContext = (SpyApplicationContext) this.context; - verify(applicationContext.getApplicationContext(), never()).registerShutdownHook(); - } - @Test void headless() { TestSpringApplication application = new TestSpringApplication(ExampleConfig.class); @@ -1344,6 +1343,7 @@ ConfigurableApplicationContext getApplicationContext() { @Override public void close() { this.applicationContext.close(); + super.close(); } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/builder/SpringApplicationBuilderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/builder/SpringApplicationBuilderTests.java index 86616b166240..c5717794498e 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/builder/SpringApplicationBuilderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/builder/SpringApplicationBuilderTests.java @@ -25,6 +25,7 @@ import org.springframework.boot.ApplicationArguments; import org.springframework.boot.ApplicationContextFactory; +import org.springframework.boot.SpringApplicationShutdownHookInstance; import org.springframework.boot.WebApplicationType; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; @@ -55,6 +56,7 @@ class SpringApplicationBuilderTests { @AfterEach void close() { close(this.context); + SpringApplicationShutdownHookInstance.reset(); } private void close(ApplicationContext context) { @@ -132,7 +134,7 @@ void parentContextCreationThatIsRunDirectly() { application.parent(ExampleConfig.class); this.context = application.run("foo.bar=baz"); verify(((SpyApplicationContext) this.context).getApplicationContext()).setParent(any(ApplicationContext.class)); - assertThat(((SpyApplicationContext) this.context).getRegisteredShutdownHook()).isFalse(); + assertThat(SpringApplicationShutdownHookInstance.get()).didNotRegisterApplicationContext(this.context); assertThat(this.context.getParent().getBean(ApplicationArguments.class).getNonOptionArgs()) .contains("foo.bar=baz"); assertThat(this.context.getBean(ApplicationArguments.class).getNonOptionArgs()).contains("foo.bar=baz"); @@ -145,7 +147,7 @@ void parentContextCreationThatIsBuiltThenRun() { application.parent(ExampleConfig.class); this.context = application.build("a=alpha").run("b=bravo"); verify(((SpyApplicationContext) this.context).getApplicationContext()).setParent(any(ApplicationContext.class)); - assertThat(((SpyApplicationContext) this.context).getRegisteredShutdownHook()).isFalse(); + assertThat(SpringApplicationShutdownHookInstance.get()).didNotRegisterApplicationContext(this.context); assertThat(this.context.getParent().getBean(ApplicationArguments.class).getNonOptionArgs()).contains("a=alpha"); assertThat(this.context.getBean(ApplicationArguments.class).getNonOptionArgs()).contains("b=bravo"); } @@ -158,7 +160,7 @@ void parentContextCreationWithChildShutdown() { application.parent(ExampleConfig.class); this.context = application.run(); verify(((SpyApplicationContext) this.context).getApplicationContext()).setParent(any(ApplicationContext.class)); - assertThat(((SpyApplicationContext) this.context).getRegisteredShutdownHook()).isTrue(); + assertThat(SpringApplicationShutdownHookInstance.get()).registeredApplicationContext(this.context); } @Test @@ -189,7 +191,7 @@ void parentFirstCreation() { application.contextFactory(ApplicationContextFactory.ofContextClass(SpyApplicationContext.class)); this.context = application.run(); verify(((SpyApplicationContext) this.context).getApplicationContext()).setParent(any(ApplicationContext.class)); - assertThat(((SpyApplicationContext) this.context).getRegisteredShutdownHook()).isFalse(); + assertThat(SpringApplicationShutdownHookInstance.get()).didNotRegisterApplicationContext(this.context); } @Test @@ -323,8 +325,6 @@ static class SpyApplicationContext extends AnnotationConfigApplicationContext { private ResourceLoader resourceLoader; - private boolean registeredShutdownHook; - @Override public void setParent(ApplicationContext parent) { this.applicationContext.setParent(parent); @@ -344,16 +344,6 @@ ResourceLoader getResourceLoader() { return this.resourceLoader; } - @Override - public void registerShutdownHook() { - super.registerShutdownHook(); - this.registeredShutdownHook = true; - } - - boolean getRegisteredShutdownHook() { - return this.registeredShutdownHook; - } - @Override public void close() { super.close(); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java index e3f02482bcf6..38a35c5dbfeb 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java @@ -415,7 +415,7 @@ void shutdownHookIsRegisteredByDefault() throws Exception { multicastEvent(listener, new ApplicationStartingEvent(this.bootstrapContext, new SpringApplication(), NO_ARGS)); listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); assertThat(listener.shutdownHook).isNotNull(); - listener.shutdownHook.start(); + listener.shutdownHook.run(); assertThat(TestShutdownHandlerLoggingSystem.shutdownLatch.await(30, TimeUnit.SECONDS)).isTrue(); } @@ -634,10 +634,10 @@ public Runnable getShutdownHandler() { static class TestLoggingApplicationListener extends LoggingApplicationListener { - private Thread shutdownHook; + private Runnable shutdownHook; @Override - void registerShutdownHook(Thread shutdownHook) { + void registerShutdownHook(Runnable shutdownHook) { this.shutdownHook = shutdownHook; }