From f08f872729a6be36247a61c7adb67b071f1a8d20 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 6 Nov 2015 11:06:13 -0800 Subject: [PATCH] Restore original embedded container shutdown order Update EmbeddedWebApplicationContext so that the servlet container is shutdown after the context is closed. Unfortunately shutting the container down before the context has been closed causes exceptions if the `/shutdown` actuator endpoint is used. It can also cause the Tomcat classloader to throw IllegalStateExceptions if resources are accessed during shutdown. As this commit effectively reverts 0069e41c we need to fix the shutdown deadlock issue reported in gh-4130 in a different way. The deadlock can be caused when an incoming HTTP connection occurs whilst the context is closing. The incoming connection triggers the `FrameworkServlet` to call `initWebApplicationContext` which in turn calls `refresh`. The `FrameworkServlet` checks `ApplicationContext.isActive()` before performing an initialization but prior to this commit we would set active to `false` before stopping the servlet container. We now override `onClose` rather than `doClose` in `EmbeddedWebApplicationContext` to ensure that the active flag is only set to `false` once the servlet container has been stopped. See gh-4130 Fixes gh-4396 --- .../EmbeddedWebApplicationContext.java | 4 +- .../EmbeddedWebApplicationContextTests.java | 41 ------------------- .../MockEmbeddedServletContainerFactory.java | 7 ---- 3 files changed, 2 insertions(+), 50 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContext.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContext.java index 3c8d3555463..418a3ddb9c8 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContext.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContext.java @@ -146,9 +146,9 @@ public class EmbeddedWebApplicationContext extends GenericWebApplicationContext } @Override - protected void doClose() { + protected void onClose() { + super.onClose(); stopAndReleaseEmbeddedServletContainer(); - super.doClose(); } private synchronized void createEmbeddedServletContainer() { diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContextTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContextTests.java index 7bf1729347d..306aa4dd32d 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContextTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContextTests.java @@ -37,14 +37,11 @@ import org.junit.rules.ExpectedException; import org.mockito.InOrder; import org.springframework.beans.MutablePropertyValues; -import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.config.ConstructorArgumentValues; import org.springframework.beans.factory.config.Scope; -import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.RootBeanDefinition; -import org.springframework.boot.context.embedded.MockEmbeddedServletContainerFactory.MockEmbeddedServletContainer; import org.springframework.context.ApplicationContextException; import org.springframework.context.ApplicationListener; import org.springframework.context.support.AbstractApplicationContext; @@ -58,7 +55,6 @@ import org.springframework.web.filter.GenericFilterBean; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; import static org.junit.Assert.assertEquals; @@ -434,22 +430,6 @@ public class EmbeddedWebApplicationContextTests { sameInstance(scope)); } - @Test - public void containerIsStoppedBeforeContextIsClosed() { - addEmbeddedServletContainerFactoryBean(); - this.context.registerBeanDefinition("shutdownOrderingValidator", - BeanDefinitionBuilder.rootBeanDefinition(ShutdownOrderingValidator.class) - .addConstructorArgReference("embeddedServletContainerFactory") - .getBeanDefinition()); - this.context.refresh(); - ShutdownOrderingValidator validator = this.context - .getBean(ShutdownOrderingValidator.class); - this.context.close(); - assertThat(validator.destroyed, is(true)); - assertThat(validator.containerStoppedFirst, is(true)); - - } - private void addEmbeddedServletContainerFactoryBean() { this.context.registerBeanDefinition("embeddedServletContainerFactory", new RootBeanDefinition(MockEmbeddedServletContainerFactory.class)); @@ -499,25 +479,4 @@ public class EmbeddedWebApplicationContextTests { } - protected static class ShutdownOrderingValidator implements DisposableBean { - - private final MockEmbeddedServletContainer servletContainer; - - private boolean destroyed = false; - - private boolean containerStoppedFirst = false; - - ShutdownOrderingValidator( - MockEmbeddedServletContainerFactory servletContainerFactory) { - this.servletContainer = servletContainerFactory.getContainer(); - } - - @Override - public void destroy() { - this.destroyed = true; - this.containerStoppedFirst = this.servletContainer.isStopped(); - } - - } - } diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/MockEmbeddedServletContainerFactory.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/MockEmbeddedServletContainerFactory.java index dcbdcdb67b7..6debacdb9ff 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/MockEmbeddedServletContainerFactory.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/MockEmbeddedServletContainerFactory.java @@ -90,8 +90,6 @@ public class MockEmbeddedServletContainerFactory private final int port; - private boolean stopped = false; - public MockEmbeddedServletContainer(ServletContextInitializer[] initializers, int port) { this.initializers = initializers; @@ -190,11 +188,6 @@ public class MockEmbeddedServletContainerFactory public void stop() { this.servletContext = null; this.registeredServlets.clear(); - this.stopped = true; - } - - public boolean isStopped() { - return this.stopped; } public Servlet[] getServlets() {