Separate stopping and destruction so web server can be restarted

Previously, when a Servlet-based WebServer was stopped it would also
stop the ServletContext. This led to problems as Tomcat and Undertow
would then not allow a restart. Jetty would allow a restart but
duplicate servlet registrations would then be attempted.

This commit modifies the WebServer lifecycle to separate stopping
and destruction for both servlet and reactive web servers. This
allows a WebServer's stop() implementation to leave some components
running so that they can be restarted. To completely shut down a
WebServer destroy() must now be called.

Both Tomcat and Jetty WebServers have been updated to stop their
network connections when stop() is called but leave other components
running. This works with both servlet and reactive web servers.

Note that an Undertow-based Servlet web server does not support
stop and restart. Once stopped, a Servlet Deployment cannot be
restarted and it does not appear to be possible to separate the
lifecycle of its network connections and a Servlet deployment.

Reactor Netty and Undertow-based reactive web servers can now also
be stopped and then restarted. Calling stop() stops the whole server
but this does not cause a problem as there's no (application-exposed)
ServletContext involved. There may be room to optimize this in the
future if the need arises.

Closes gh-34955

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyWebServer.java

@@ -238,7 +238,9 @@ public class JettyWebServer implements WebServer {
 				this.gracefulShutdown.abort();
 			}
 			try {
-				this.server.stop();
+				for (Connector connector : this.server.getConnectors()) {
+					connector.stop();
+				}
 			}
 			catch (InterruptedException ex) {
 				Thread.currentThread().interrupt();
@@ -249,6 +251,18 @@ public class JettyWebServer implements WebServer {
 		}
 	}
 
+	@Override
+	public void destroy() {
+		synchronized (this.monitor) {
+			try {
+				this.server.stop();
+			}
+			catch (Exception ex) {
+				throw new WebServerException("Unable to destroy embedded Jetty server", ex);
+			}
+		}
+	}
+
 	@Override
 	public int getPort() {
 		Connector[] connectors = this.server.getConnectors();

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/GracefulShutdown.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2020 the original author or authors.
+ * Copyright 2012-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -60,14 +60,14 @@ final class GracefulShutdown {
 		try {
 			for (Container host : this.tomcat.getEngine().findChildren()) {
 				for (Container context : host.findChildren()) {
-					while (isActive(context)) {
-						if (this.aborted) {
-							logger.info("Graceful shutdown aborted with one or more requests still active");
-							callback.shutdownComplete(GracefulShutdownResult.REQUESTS_ACTIVE);
-							return;
-						}
+					while (!this.aborted && isActive(context)) {
 						Thread.sleep(50);
 					}
+					if (this.aborted) {
+						logger.info("Graceful shutdown aborted with one or more requests still active");
+						callback.shutdownComplete(GracefulShutdownResult.REQUESTS_ACTIVE);
+						return;
+					}
 				}
 			}
 

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java

@@ -209,6 +209,7 @@ public class TomcatWebServer implements WebServer {
 			if (this.started) {
 				return;
 			}
+
 			try {
 				addPreviouslyRemovedConnectors();
 				Connector connector = this.tomcat.getConnector();
@@ -324,16 +325,10 @@ public class TomcatWebServer implements WebServer {
 			boolean wasStarted = this.started;
 			try {
 				this.started = false;
-				try {
-					if (this.gracefulShutdown != null) {
-						this.gracefulShutdown.abort();
-					}
-					stopTomcat();
-					this.tomcat.destroy();
-				}
-				catch (LifecycleException ex) {
-					// swallow and continue
+				if (this.gracefulShutdown != null) {
+					this.gracefulShutdown.abort();
 				}
+				removeServiceConnectors();
 			}
 			catch (Exception ex) {
 				throw new WebServerException("Unable to stop embedded Tomcat", ex);
@@ -346,6 +341,19 @@ public class TomcatWebServer implements WebServer {
 		}
 	}
 
+	public void destroy() throws WebServerException {
+		try {
+			stopTomcat();
+			this.tomcat.destroy();
+		}
+		catch (LifecycleException ex) {
+			// Swallow and continue
+		}
+		catch (Exception ex) {
+			throw new WebServerException("Unable to destroy embedded Tomcat", ex);
+		}
+	}
+
 	private String getPortsDescription(boolean localPort) {
 		StringBuilder ports = new StringBuilder();
 		for (Connector connector : this.tomcat.getService().findConnectors()) {

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/undertow/UndertowWebServer.java

@@ -132,13 +132,13 @@ public class UndertowWebServer implements WebServer {
 					throw new WebServerException("Unable to start embedded Undertow", ex);
 				}
 				finally {
-					stopSilently();
+					destroySilently();
 				}
 			}
 		}
 	}
 
-	private void stopSilently() {
+	private void destroySilently() {
 		try {
 			if (this.undertow != null) {
 				this.undertow.stop();
@@ -274,7 +274,7 @@ public class UndertowWebServer implements WebServer {
 				}
 			}
 			catch (Exception ex) {
-				throw new WebServerException("Unable to stop undertow", ex);
+				throw new WebServerException("Unable to stop Undertow", ex);
 			}
 		}
 	}

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/server/WebServer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2020 the original author or authors.
+ * Copyright 2012-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -61,4 +61,12 @@ public interface WebServer {
 		callback.shutdownComplete(GracefulShutdownResult.IMMEDIATE);
 	}
 
+	/**
+	 * Destroys the web server such that it cannot be started again.
+	 * @since 3.2.0
+	 */
+	default void destroy() {
+		stop();
+	}
+
 }

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2022 the original author or authors.
+ * Copyright 2012-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -149,6 +149,7 @@ public class ServletWebServerApplicationContext extends GenericWebApplicationCon
 			WebServer webServer = this.webServer;
 			if (webServer != null) {
 				webServer.stop();
+				webServer.destroy();
 			}
 			throw ex;
 		}
@@ -171,6 +172,10 @@ public class ServletWebServerApplicationContext extends GenericWebApplicationCon
 			AvailabilityChangeEvent.publish(this, ReadinessState.REFUSING_TRAFFIC);
 		}
 		super.doClose();
+		WebServer webServer = this.webServer;
+		if (webServer != null) {
+			webServer.destroy();
+		}
 	}
 
 	private void createWebServer() {

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactoryTests.java

@@ -234,10 +234,10 @@ class JettyServletWebServerFactoryTests extends AbstractServletWebServerFactoryT
 	}
 
 	@Test
-	void stopCalledWithoutStart() {
+	void destroyCalledWithoutStart() {
 		JettyServletWebServerFactory factory = getFactory();
 		this.webServer = factory.getWebServer(exampleServletRegistration());
-		this.webServer.stop();
+		this.webServer.destroy();
 		Server server = ((JettyWebServer) this.webServer).getServer();
 		assertThat(server.isStopped()).isTrue();
 	}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java

@@ -340,10 +340,10 @@ class TomcatServletWebServerFactoryTests extends AbstractServletWebServerFactory
 	}
 
 	@Test
-	void stopCalledWithoutStart() {
+	void destroyCalledWithoutStart() {
 		TomcatServletWebServerFactory factory = getFactory();
 		this.webServer = factory.getWebServer(exampleServletRegistration());
-		this.webServer.stop();
+		this.webServer.destroy();
 		Tomcat tomcat = ((TomcatWebServer) this.webServer).getTomcat();
 		assertThat(tomcat.getServer().getState()).isSameAs(LifecycleState.DESTROYED);
 	}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/undertow/UndertowServletWebServerFactoryTests.java

@@ -40,6 +40,7 @@ import org.apache.hc.core5.http.HttpResponse;
 import org.apache.jasper.servlet.JspServlet;
 import org.awaitility.Awaitility;
 import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.mockito.InOrder;
 
@@ -211,6 +212,20 @@ class UndertowServletWebServerFactoryTests extends AbstractServletWebServerFacto
 		this.webServer.stop();
 	}
 
+	@Test
+	@Override
+	@Disabled("Restart after stop is not supported with Undertow")
+	protected void restartAfterStop() {
+
+	}
+
+	@Test
+	@Override
+	@Disabled("Undertow's architecture prevents separating stop and destroy")
+	protected void servletContextListenerContextDestroyedIsNotCalledWhenContainerIsStopped() {
+
+	}
+
 	private void testAccessLog(String prefix, String suffix, String expectedFile)
 			throws IOException, URISyntaxException {
 		UndertowServletWebServerFactory factory = getFactory();

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java

@@ -77,6 +77,7 @@ import org.springframework.web.reactive.function.client.WebClient;
 import org.springframework.web.reactive.function.client.WebClientRequestException;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatException;
 import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
@@ -95,6 +96,12 @@ public abstract class AbstractReactiveWebServerFactoryTests {
 		if (this.webServer != null) {
 			try {
 				this.webServer.stop();
+				try {
+					this.webServer.destroy();
+				}
+				catch (Exception ex) {
+					// Ignore
+				}
 			}
 			catch (Exception ex) {
 				// Ignore
@@ -124,13 +131,37 @@ public abstract class AbstractReactiveWebServerFactoryTests {
 		assertThat(this.webServer.getPort()).isEqualTo(specificPort);
 	}
 
+	@Test
+	protected void restartAfterStop() throws Exception {
+		AbstractReactiveWebServerFactory factory = getFactory();
+		this.webServer = factory.getWebServer(new EchoHandler());
+		this.webServer.start();
+		int port = this.webServer.getPort();
+		assertThat(getResponse(port, "/test")).isEqualTo("Hello World");
+		this.webServer.stop();
+		assertThatException().isThrownBy(() -> getResponse(port, "/test"));
+		this.webServer.start();
+		assertThat(getResponse(this.webServer.getPort(), "/test")).isEqualTo("Hello World");
+	}
+
+	private String getResponse(int port, String uri) {
+		WebClient webClient = getWebClient(port).build();
+		Mono<String> result = webClient.post()
+			.uri(uri)
+			.contentType(MediaType.TEXT_PLAIN)
+			.body(BodyInserters.fromValue("Hello World"))
+			.retrieve()
+			.bodyToMono(String.class);
+		return result.block(Duration.ofSeconds(30));
+	}
+
 	@Test
 	void portIsMinusOneWhenConnectionIsClosed() {
 		AbstractReactiveWebServerFactory factory = getFactory();
 		this.webServer = factory.getWebServer(new EchoHandler());
 		this.webServer.start();
 		assertThat(this.webServer.getPort()).isGreaterThan(0);
-		this.webServer.stop();
+		this.webServer.destroy();
 		assertThat(this.webServer.getPort()).isEqualTo(-1);
 	}
 

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContextTests.java

@@ -84,6 +84,7 @@ import static org.mockito.BDDMockito.then;
 import static org.mockito.Mockito.atMost;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.withSettings;
 
 /**
@@ -156,12 +157,35 @@ class ServletWebServerApplicationContextTests {
 	}
 
 	@Test
-	void stopOnClose() {
+	void stopOnStop() {
 		addWebServerFactoryBean();
 		this.context.refresh();
 		MockServletWebServerFactory factory = getWebServerFactory();
-		this.context.close();
+		then(factory.getWebServer()).should().start();
+		this.context.stop();
+		then(factory.getWebServer()).should().stop();
+	}
+
+	@Test
+	void startOnStartAfterStop() {
+		addWebServerFactoryBean();
+		this.context.refresh();
+		MockServletWebServerFactory factory = getWebServerFactory();
+		then(factory.getWebServer()).should().start();
+		this.context.stop();
 		then(factory.getWebServer()).should().stop();
+		this.context.start();
+		then(factory.getWebServer()).should(times(2)).start();
+	}
+
+	@Test
+	void stopAndDestroyOnClose() {
+		addWebServerFactoryBean();
+		this.context.refresh();
+		MockServletWebServerFactory factory = getWebServerFactory();
+		this.context.close();
+		then(factory.getWebServer()).should(times(2)).stop();
+		then(factory.getWebServer()).should().destroy();
 	}
 
 	@Test

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java

@@ -167,6 +167,7 @@ import static org.mockito.BDDMockito.then;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 
 /**
  * Base for testing classes that extends {@link AbstractServletWebServerFactory}.
@@ -197,6 +198,12 @@ public abstract class AbstractServletWebServerFactoryTests {
 		if (this.webServer != null) {
 			try {
 				this.webServer.stop();
+				try {
+					this.webServer.destroy();
+				}
+				catch (Exception ex) {
+					// Ignore
+				}
 			}
 			catch (Exception ex) {
 				// Ignore
@@ -233,6 +240,19 @@ public abstract class AbstractServletWebServerFactoryTests {
 		this.webServer.stop();
 	}
 
+	@Test
+	protected void restartAfterStop() throws IOException, URISyntaxException {
+		AbstractServletWebServerFactory factory = getFactory();
+		this.webServer = factory.getWebServer(exampleServletRegistration());
+		this.webServer.start();
+		assertThat(getResponse(getLocalUrl("/hello"))).isEqualTo("Hello World");
+		int port = this.webServer.getPort();
+		this.webServer.stop();
+		assertThatIOException().isThrownBy(() -> getResponse(getLocalUrl(port, "/hello")));
+		this.webServer.start();
+		assertThat(getResponse(getLocalUrl("/hello"))).isEqualTo("Hello World");
+	}
+
 	@Test
 	void emptyServerWhenPortIsMinusOne() {
 		AbstractServletWebServerFactory factory = getFactory();
@@ -295,7 +315,7 @@ public abstract class AbstractServletWebServerFactoryTests {
 		this.webServer = factory.getWebServer();
 		this.webServer.start();
 		assertThat(this.webServer.getPort()).isGreaterThan(0);
-		this.webServer.stop();
+		this.webServer.destroy();
 		assertThat(this.webServer.getPort()).isEqualTo(-1);
 	}
 
@@ -814,7 +834,7 @@ public abstract class AbstractServletWebServerFactoryTests {
 		this.webServer.start();
 		String s1 = getResponse(getLocalUrl("/session"));
 		String s2 = getResponse(getLocalUrl("/session"));
-		this.webServer.stop();
+		this.webServer.destroy();
 		this.webServer = factory.getWebServer(sessionServletRegistration());
 		this.webServer.start();
 		String s3 = getResponse(getLocalUrl("/session"));
@@ -833,7 +853,7 @@ public abstract class AbstractServletWebServerFactoryTests {
 		this.webServer = factory.getWebServer(sessionServletRegistration());
 		this.webServer.start();
 		getResponse(getLocalUrl("/session"));
-		this.webServer.stop();
+		this.webServer.destroy();
 		File[] dirContents = sessionStoreDir.listFiles((dir, name) -> !(".".equals(name) || "..".equals(name)));
 		assertThat(dirContents).isNotEmpty();
 	}
@@ -1158,11 +1178,20 @@ public abstract class AbstractServletWebServerFactoryTests {
 	}
 
 	@Test
-	void servletContextListenerContextDestroyedIsCalledWhenContainerIsStopped() throws Exception {
+	protected void servletContextListenerContextDestroyedIsNotCalledWhenContainerIsStopped() throws Exception {
 		ServletContextListener listener = mock(ServletContextListener.class);
 		this.webServer = getFactory().getWebServer((servletContext) -> servletContext.addListener(listener));
 		this.webServer.start();
 		this.webServer.stop();
+		then(listener).should(times(0)).contextDestroyed(any(ServletContextEvent.class));
+	}
+
+	@Test
+	void servletContextListenerContextDestroyedIsCalledWhenContainerIsDestroyed() throws Exception {
+		ServletContextListener listener = mock(ServletContextListener.class);
+		this.webServer = getFactory().getWebServer((servletContext) -> servletContext.addListener(listener));
+		this.webServer.start();
+		this.webServer.destroy();
 		then(listener).should().contextDestroyed(any(ServletContextEvent.class));
 	}
 

spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-app/src/main/java/org/springframework/boot/loaderapp/LoaderTestApplication.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2021 the original author or authors.
+ * Copyright 2012-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -53,7 +53,7 @@ public class LoaderTestApplication {
 	}
 
 	public static void main(String[] args) {
-		SpringApplication.run(LoaderTestApplication.class, args).stop();
+		SpringApplication.run(LoaderTestApplication.class, args).close();
 	}
 
 }