Skip to content

Commit

Permalink
Separate stopping and destruction so web server can be restarted
Browse files Browse the repository at this point in the history
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
  • Loading branch information
wilkinsona committed Jun 26, 2023
1 parent 47cc65d commit dbb2428
Show file tree
Hide file tree
Showing 13 changed files with 169 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,9 @@ public void stop() {
this.gracefulShutdown.abort();
}
try {
this.server.stop();
for (Connector connector : this.server.getConnectors()) {
connector.stop();
}
}
catch (InterruptedException ex) {
Thread.currentThread().interrupt();
Expand All @@ -249,6 +251,18 @@ public void stop() {
}
}

@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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -60,14 +60,14 @@ private void doShutdown(GracefulShutdownCallback callback) {
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;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ public void start() throws WebServerException {
if (this.started) {
return;
}

try {
addPreviouslyRemovedConnectors();
Connector connector = this.tomcat.getConnector();
Expand Down Expand Up @@ -324,16 +325,10 @@ public void stop() throws WebServerException {
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);
Expand All @@ -346,6 +341,19 @@ public void stop() throws WebServerException {
}
}

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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,13 @@ public void start() throws WebServerException {
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();
Expand Down Expand Up @@ -274,7 +274,7 @@ public void stop() throws WebServerException {
}
}
catch (Exception ex) {
throw new WebServerException("Unable to stop undertow", ex);
throw new WebServerException("Unable to stop Undertow", ex);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -61,4 +61,12 @@ default void shutDownGracefully(GracefulShutdownCallback callback) {
callback.shutdownComplete(GracefulShutdownResult.IMMEDIATE);
}

/**
* Destroys the web server such that it cannot be started again.
* @since 3.2.0
*/
default void destroy() {
stop();
}

}
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -149,6 +149,7 @@ public final void refresh() throws BeansException, IllegalStateException {
WebServer webServer = this.webServer;
if (webServer != null) {
webServer.stop();
webServer.destroy();
}
throw ex;
}
Expand All @@ -171,6 +172,10 @@ protected void doClose() {
AvailabilityChangeEvent.publish(this, ReadinessState.REFUSING_TRAFFIC);
}
super.doClose();
WebServer webServer = this.webServer;
if (webServer != null) {
webServer.destroy();
}
}

private void createWebServer() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,10 +234,10 @@ void sslCiphersConfiguration() {
}

@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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,10 +340,10 @@ void startupFailureDoesNotResultInUnstoppedThreadsBeingReported(CapturedOutput o
}

@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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
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;

Expand Down Expand Up @@ -211,6 +212,20 @@ void whenServerIsShuttingDownGracefullyThenRequestsAreRejectedWithServiceUnavail
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
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;

Expand All @@ -95,6 +96,12 @@ void tearDown() {
if (this.webServer != null) {
try {
this.webServer.stop();
try {
this.webServer.destroy();
}
catch (Exception ex) {
// Ignore
}
}
catch (Exception ex) {
// Ignore
Expand Down Expand Up @@ -124,13 +131,37 @@ void specificPort() throws Exception {
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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
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;

/**
Expand Down Expand Up @@ -156,12 +157,35 @@ void localPortIsAvailable() {
}

@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
Expand Down
Loading

0 comments on commit dbb2428

Please sign in to comment.