Detect startup failures in additional Tomcat connectors

Previously, only the state of the primary connector was checked when
verifying that the embedded Tomcat instance has started successfully.
This commit updates the verification logic to examine all of the
service's connectors, thereby also detecting startup failures of any
additional connectors the have been configured.

A check on the primary connector's state has been removed from
initialize as, at this stage, its state will always be NEW.

Fixes gh-1591

Author: wilkinsona

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

@@ -90,11 +90,6 @@ private synchronized void initialize() throws EmbeddedServletContainerException
 			// Unlike Jetty, all Tomcat threads are daemon threads. We create a
 			// blocking non-daemon to stop immediate shutdown
 			startDaemonAwaitThread();
-
-			if (LifecycleState.FAILED.equals(this.tomcat.getConnector().getState())) {
-				this.tomcat.stop();
-				throw new IllegalStateException("Tomcat connector in failed state");
-			}
 		}
 		catch (Exception ex) {
 			throw new EmbeddedServletContainerException(
@@ -151,13 +146,24 @@ public void start() throws EmbeddedServletContainerException {
 		if (connector != null && this.autoStart) {
 			startConnector(connector);
 		}
+
 		// Ensure process isn't left running if it actually failed to start
-		if (LifecycleState.FAILED.equals(this.tomcat.getConnector().getState())) {
+
+		if (connectorsHaveFailedToStart()) {
 			stopSilently();
 			throw new IllegalStateException("Tomcat connector in failed state");
 		}
 	}
 
+	private boolean connectorsHaveFailedToStart() {
+		for (Connector connector : this.tomcat.getService().findConnectors()) {
+			if (LifecycleState.FAILED.equals(connector.getState())) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	private void stopSilently() {
 		try {
 			this.tomcat.stop();

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

@@ -16,6 +16,10 @@
 
 package org.springframework.boot.context.embedded.tomcat;
 
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.net.ServerSocket;
+import java.net.UnknownHostException;
 import java.util.Arrays;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
@@ -39,6 +43,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyObject;
@@ -248,6 +253,63 @@ public void sslCiphersConfiguration() throws Exception {
 		assertThat(jsseProtocol.getCiphers(), equalTo("ALPHA,BRAVO,CHARLIE"));
 	}
 
+	@Test
+	public void primaryConnectorPortClashThrowsIllegalStateException()
+			throws InterruptedException, UnknownHostException, IOException {
+		final int port = SocketUtils.findAvailableTcpPort(40000);
+
+		doWithBlockedPort(port, new Runnable() {
+
+			@Override
+			public void run() {
+				TomcatEmbeddedServletContainerFactory factory = getFactory();
+				factory.setPort(port);
+
+				try {
+					TomcatEmbeddedServletContainerFactoryTests.this.container = factory
+							.getEmbeddedServletContainer();
+					TomcatEmbeddedServletContainerFactoryTests.this.container.start();
+					fail();
+				}
+				catch (IllegalStateException ex) {
+
+				}
+			}
+
+		});
+
+	}
+
+	@Test
+	public void additionalConnectorPortClashThrowsIllegalStateException()
+			throws InterruptedException, UnknownHostException, IOException {
+		final int port = SocketUtils.findAvailableTcpPort(40000);
+
+		doWithBlockedPort(port, new Runnable() {
+
+			@Override
+			public void run() {
+				TomcatEmbeddedServletContainerFactory factory = getFactory();
+				Connector connector = new Connector(
+						"org.apache.coyote.http11.Http11NioProtocol");
+				connector.setPort(port);
+				factory.addAdditionalTomcatConnectors(connector);
+
+				try {
+					TomcatEmbeddedServletContainerFactoryTests.this.container = factory
+							.getEmbeddedServletContainer();
+					TomcatEmbeddedServletContainerFactoryTests.this.container.start();
+					fail();
+				}
+				catch (IllegalStateException ex) {
+
+				}
+			}
+
+		});
+
+	}
+
 	private void assertTimeout(TomcatEmbeddedServletContainerFactory factory, int expected) {
 		Tomcat tomcat = getTomcat(factory);
 		Context context = (Context) tomcat.getHost().findChildren()[0];
@@ -259,4 +321,16 @@ private Tomcat getTomcat(TomcatEmbeddedServletContainerFactory factory) {
 		return ((TomcatEmbeddedServletContainer) this.container).getTomcat();
 	}
 
+	private void doWithBlockedPort(final int port, Runnable action) throws IOException {
+		ServerSocket serverSocket = new ServerSocket();
+		serverSocket.bind(new InetSocketAddress(port));
+
+		try {
+			action.run();
+		}
+		finally {
+			serverSocket.close();
+		}
+	}
+
 }