Ensure embedded start can be called multiple times

Update all `EmbeddedServletContainer` implementations to ensure that the `start()` method can be called multiple times, with the second call being ignored. Fixes gh-8036
spring-projects · Jan 26, 2017 · 0af53b3 · 0af53b3
1 parent ef69ae6
commit 0af53b3
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 58 deletions.
diff --git a/...n/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java b/...n/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java
@@ -59,6 +59,8 @@ public class JettyEmbeddedServletContainer implements EmbeddedServletContainer {
 
 	private Connector[] connectors;
 
+	private volatile boolean started;
+
 	/**
 	 * Create a new {@link JettyEmbeddedServletContainer} instance.
 	 * @param server the underlying Jetty server
@@ -111,37 +113,43 @@ private void stopSilently() {
 
 	@Override
 	public void start() throws EmbeddedServletContainerException {
-		this.server.setConnectors(this.connectors);
-		if (!this.autoStart) {
-			return;
-		}
-		try {
-			this.server.start();
-			for (Handler handler : this.server.getHandlers()) {
-				handleDeferredInitialize(handler);
+		synchronized (this.monitor) {
+			if (this.started) {
+				return;
+			}
+			this.server.setConnectors(this.connectors);
+			if (!this.autoStart) {
+				return;
 			}
-			Connector[] connectors = this.server.getConnectors();
-			for (Connector connector : connectors) {
-				try {
-					connector.start();
+			try {
+				this.server.start();
+				for (Handler handler : this.server.getHandlers()) {
+					handleDeferredInitialize(handler);
 				}
-				catch (BindException ex) {
-					if (connector instanceof NetworkConnector) {
-						throw new PortInUseException(
-								((NetworkConnector) connector).getPort());
+				Connector[] connectors = this.server.getConnectors();
+				for (Connector connector : connectors) {
+					try {
+						connector.start();
+					}
+					catch (BindException ex) {
+						if (connector instanceof NetworkConnector) {
+							throw new PortInUseException(
+									((NetworkConnector) connector).getPort());
+						}
+						throw ex;
 					}
-					throw ex;
 				}
+				this.started = true;
+				JettyEmbeddedServletContainer.logger
+						.info("Jetty started on port(s) " + getActualPortsDescription());
+			}
+			catch (EmbeddedServletContainerException ex) {
+				throw ex;
+			}
+			catch (Exception ex) {
+				throw new EmbeddedServletContainerException(
+						"Unable to start embedded Jetty servlet container", ex);
 			}
-			JettyEmbeddedServletContainer.logger
-					.info("Jetty started on port(s) " + getActualPortsDescription());
-		}
-		catch (EmbeddedServletContainerException ex) {
-			throw ex;
-		}
-		catch (Exception ex) {
-			throw new EmbeddedServletContainerException(
-					"Unable to start embedded Jetty servlet container", ex);
 		}
 	}
 
@@ -197,6 +205,10 @@ else if (handler instanceof HandlerCollection) {
 	@Override
 	public void stop() {
 		synchronized (this.monitor) {
+			if (!this.started) {
+				return;
+			}
+			this.started = false;
 			try {
 				this.server.stop();
 			}

diff --git a/...java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java b/...java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java
@@ -62,6 +62,8 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer
 
 	private final boolean autoStart;
 
+	private volatile boolean started;
+
 	/**
 	 * Create a new {@link TomcatEmbeddedServletContainer} instance.
 	 * @param tomcat the underlying Tomcat server
@@ -174,28 +176,34 @@ public void run() {
 
 	@Override
 	public void start() throws EmbeddedServletContainerException {
-		try {
-			addPreviouslyRemovedConnectors();
-			Connector connector = this.tomcat.getConnector();
-			if (connector != null && this.autoStart) {
-				startConnector(connector);
+		synchronized (this.monitor) {
+			if (this.started) {
+				return;
+			}
+			try {
+				addPreviouslyRemovedConnectors();
+				Connector connector = this.tomcat.getConnector();
+				if (connector != null && this.autoStart) {
+					startConnector(connector);
+				}
+				checkThatConnectorsHaveStarted();
+				this.started = true;
+				TomcatEmbeddedServletContainer.logger
+						.info("Tomcat started on port(s): " + getPortsDescription(true));
+			}
+			catch (ConnectorStartFailedException ex) {
+				stopSilently();
+				throw ex;
+			}
+			catch (Exception ex) {
+				throw new EmbeddedServletContainerException(
+						"Unable to start embedded Tomcat servlet container", ex);
+			}
+			finally {
+				Context context = findContext();
+				ContextBindings.unbindClassLoader(context, getNamingToken(context),
+						getClass().getClassLoader());
 			}
-			checkThatConnectorsHaveStarted();
-			TomcatEmbeddedServletContainer.logger
-					.info("Tomcat started on port(s): " + getPortsDescription(true));
-		}
-		catch (ConnectorStartFailedException ex) {
-			stopSilently();
-			throw ex;
-		}
-		catch (Exception ex) {
-			throw new EmbeddedServletContainerException(
-					"Unable to start embedded Tomcat servlet container", ex);
-		}
-		finally {
-			Context context = findContext();
-			ContextBindings.unbindClassLoader(context, getNamingToken(context),
-					getClass().getClassLoader());
 		}
 	}
 
@@ -271,7 +279,11 @@ Map<Service, Connector[]> getServiceConnectors() {
 	@Override
 	public void stop() throws EmbeddedServletContainerException {
 		synchronized (this.monitor) {
+			if (!this.started) {
+				return;
+			}
 			try {
+				this.started = false;
 				try {
 					stopTomcat();
 					this.tomcat.destroy();

diff --git a/.../org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainer.java b/.../org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainer.java
@@ -88,7 +88,7 @@ public class UndertowEmbeddedServletContainer implements EmbeddedServletContaine
 
 	private Undertow undertow;
 
-	private boolean started = false;
+	private volatile boolean started = false;
 
 	/**
 	 * Create a new {@link UndertowEmbeddedServletContainer} instance.
@@ -201,6 +201,9 @@ public UndertowEmbeddedServletContainer(Builder builder, DeploymentManager manag
 	@Override
 	public void start() throws EmbeddedServletContainerException {
 		synchronized (this.monitor) {
+			if (this.started) {
+				return;
+			}
 			try {
 				if (!this.autoStart) {
 					return;
@@ -362,16 +365,17 @@ private Port getPortFromListener(Object listener) {
 	@Override
 	public void stop() throws EmbeddedServletContainerException {
 		synchronized (this.monitor) {
-			if (this.started) {
-				try {
-					this.started = false;
-					this.manager.stop();
-					this.undertow.stop();
-				}
-				catch (Exception ex) {
-					throw new EmbeddedServletContainerException("Unable to stop undertow",
-							ex);
-				}
+			if (!this.started) {
+				return;
+			}
+			this.started = false;
+			try {
+				this.manager.stop();
+				this.undertow.stop();
+			}
+			catch (Exception ex) {
+				throw new EmbeddedServletContainerException("Unable to stop undertow",
+						ex);
 			}
 		}
 	}

diff --git a/...g/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java b/...g/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java
@@ -79,6 +79,7 @@
 import org.springframework.boot.ApplicationHome;
 import org.springframework.boot.ApplicationTemp;
 import org.springframework.boot.context.embedded.Ssl.ClientAuth;
+import org.springframework.boot.testutil.InternalOutputCapture;
 import org.springframework.boot.web.servlet.ErrorPage;
 import org.springframework.boot.web.servlet.FilterRegistrationBean;
 import org.springframework.boot.web.servlet.ServletContextInitializer;
@@ -120,6 +121,9 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests {
 	@Rule
 	public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
+	@Rule
+	public InternalOutputCapture output = new InternalOutputCapture();
+
 	protected EmbeddedServletContainer container;
 
 	private final HttpClientContext httpClientContext = HttpClientContext.create();
@@ -153,6 +157,19 @@ public void startServlet() throws Exception {
 		assertThat(getResponse(getLocalUrl("/hello"))).isEqualTo("Hello World");
 	}
 
+	@Test
+	public void startCalledTwice() throws Exception {
+		AbstractEmbeddedServletContainerFactory factory = getFactory();
+		this.container = factory
+				.getEmbeddedServletContainer(exampleServletRegistration());
+		this.container.start();
+		int port = this.container.getPort();
+		this.container.start();
+		assertThat(this.container.getPort()).isEqualTo(port);
+		assertThat(getResponse(getLocalUrl("/hello"))).isEqualTo("Hello World");
+		assertThat(this.output.toString()).containsOnlyOnce("started on port");
+	}
+
 	@Test
 	public void emptyServerWhenPortIsMinusOne() throws Exception {
 		AbstractEmbeddedServletContainerFactory factory = getFactory();