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 0069e41 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

Author: philwebb

spring-boot/src/main/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContext.java

@@ -146,9 +146,9 @@ protected void finishRefresh() {
 	}
 
 	@Override
-	protected void doClose() {
+	protected void onClose() {
+		super.onClose();
 		stopAndReleaseEmbeddedServletContainer();
-		super.doClose();
 	}
 
 	private synchronized void createEmbeddedServletContainer() {

spring-boot/src/test/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContextTests.java

@@ -37,14 +37,11 @@
 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 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 void doesNotReplaceExistingScopes() throws Exception { // gh-2082
 				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 void doFilter(ServletRequest request, ServletResponse response,
 
 	}
 
-	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();
-		}
-
-	}
-
 }

spring-boot/src/test/java/org/springframework/boot/context/embedded/MockEmbeddedServletContainerFactory.java

@@ -90,8 +90,6 @@ public static class MockEmbeddedServletContainer implements EmbeddedServletConta
 
 		private final int port;
 
-		private boolean stopped = false;
-
 		public MockEmbeddedServletContainer(ServletContextInitializer[] initializers,
 				int port) {
 			this.initializers = initializers;
@@ -190,11 +188,6 @@ public void start() throws EmbeddedServletContainerException {
 		public void stop() {
 			this.servletContext = null;
 			this.registeredServlets.clear();
-			this.stopped = true;
-		}
-
-		public boolean isStopped() {
-			return this.stopped;
 		}
 
 		public Servlet[] getServlets() {