Merge pull request #8227 from mkw/gh-8224

* pr/8227:
  Refine engine counter logic
  Polish web containers stop contribution
  Ensure web containers are stopped after close

Author: philwebb

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

@@ -205,9 +205,6 @@ else if (handler instanceof HandlerCollection) {
 	@Override
 	public void stop() {
 		synchronized (this.monitor) {
-			if (!this.started) {
-				return;
-			}
 			this.started = false;
 			try {
 				this.server.stop();

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

@@ -90,28 +90,34 @@ private void initialize() throws EmbeddedServletContainerException {
 		synchronized (this.monitor) {
 			try {
 				addInstanceIdToEngineName();
+				try {
+					// Remove service connectors to that protocol binding doesn't happen
+					// yet
+					removeServiceConnectors();
 
-				// Remove service connectors to that protocol binding doesn't happen yet
-				removeServiceConnectors();
+					// Start the server to trigger initialization listeners
+					this.tomcat.start();
 
-				// Start the server to trigger initialization listeners
-				this.tomcat.start();
+					// We can re-throw failure exception directly in the main thread
+					rethrowDeferredStartupExceptions();
 
-				// We can re-throw failure exception directly in the main thread
-				rethrowDeferredStartupExceptions();
+					Context context = findContext();
+					try {
+						ContextBindings.bindClassLoader(context, getNamingToken(context),
+								getClass().getClassLoader());
+					}
+					catch (NamingException ex) {
+						// Naming is not enabled. Continue
+					}
 
-				Context context = findContext();
-				try {
-					ContextBindings.bindClassLoader(context, getNamingToken(context),
-							getClass().getClassLoader());
+					// Unlike Jetty, all Tomcat threads are daemon threads. We create a
+					// blocking non-daemon to stop immediate shutdown
+					startDaemonAwaitThread();
 				}
-				catch (NamingException ex) {
-					// Naming is not enabled. Continue
+				catch (Exception ex) {
+					containerCounter.decrementAndGet();
+					throw ex;
 				}
-
-				// Unlike Jetty, all Tomcat threads are daemon threads. We create a
-				// blocking non-daemon to stop immediate shutdown
-				startDaemonAwaitThread();
 			}
 			catch (Exception ex) {
 				throw new EmbeddedServletContainerException(
@@ -279,9 +285,7 @@ Map<Service, Connector[]> getServiceConnectors() {
 	@Override
 	public void stop() throws EmbeddedServletContainerException {
 		synchronized (this.monitor) {
-			if (!this.started) {
-				return;
-			}
+			boolean wasStarted = this.started;
 			try {
 				this.started = false;
 				try {
@@ -297,7 +301,9 @@ public void stop() throws EmbeddedServletContainerException {
 						"Unable to stop embedded Tomcat", ex);
 			}
 			finally {
-				containerCounter.decrementAndGet();
+				if (wasStarted) {
+					containerCounter.decrementAndGet();
+				}
 			}
 		}
 	}

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

@@ -170,6 +170,16 @@ public void startCalledTwice() throws Exception {
 		assertThat(this.output.toString()).containsOnlyOnce("started on port");
 	}
 
+	@Test
+	public void stopCalledTwice() throws Exception {
+		AbstractEmbeddedServletContainerFactory factory = getFactory();
+		this.container = factory
+				.getEmbeddedServletContainer(exampleServletRegistration());
+		this.container.start();
+		this.container.stop();
+		this.container.stop();
+	}
+
 	@Test
 	public void emptyServerWhenPortIsMinusOne() throws Exception {
 		AbstractEmbeddedServletContainerFactory factory = getFactory();
@@ -311,16 +321,6 @@ public void contextRootPathMustNotBeSlash() throws Exception {
 		getFactory().setContextPath("/");
 	}
 
-	@Test
-	public void doubleStop() throws Exception {
-		AbstractEmbeddedServletContainerFactory factory = getFactory();
-		this.container = factory
-				.getEmbeddedServletContainer(exampleServletRegistration());
-		this.container.start();
-		this.container.stop();
-		this.container.stop();
-	}
-
 	@Test
 	public void multipleConfigurations() throws Exception {
 		AbstractEmbeddedServletContainerFactory factory = getFactory();

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

@@ -143,6 +143,16 @@ public void sslCiphersConfiguration() throws Exception {
 				.isEmpty();
 	}
 
+	@Test
+	public void stopCalledWithoutStart() throws Exception {
+		JettyEmbeddedServletContainerFactory factory = getFactory();
+		this.container = factory
+				.getEmbeddedServletContainer(exampleServletRegistration());
+		this.container.stop();
+		Server server = ((JettyEmbeddedServletContainer) this.container).getServer();
+		assertThat(server.isStopped()).isTrue();
+	}
+
 	@Override
 	protected void addConnector(final int port,
 			AbstractEmbeddedServletContainerFactory factory) {

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

@@ -352,6 +352,16 @@ public void startupFailureDoesNotResultInUnstoppedThreadsBeingReported()
 				.doesNotContain("appears to have started a thread named [main]");
 	}
 
+	@Test
+	public void stopCalledWithoutStart() throws Exception {
+		TomcatEmbeddedServletContainerFactory factory = getFactory();
+		this.container = factory
+				.getEmbeddedServletContainer(exampleServletRegistration());
+		this.container.stop();
+		Tomcat tomcat = ((TomcatEmbeddedServletContainer) this.container).getTomcat();
+		assertThat(tomcat.getServer().getState()).isSameAs(LifecycleState.DESTROYED);
+	}
+
 	@Override
 	protected void addConnector(int port,
 			AbstractEmbeddedServletContainerFactory factory) {