From 5f3088ed3133b07ca021ec1510e0e03098d3a28b Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 23 May 2017 21:42:16 +0100 Subject: [PATCH] Ensure that Jetty fails to start when its thread pool is misconfigured Previously, if Jetty's thread pool was misconfigured, the Server would still start successfully. When it is started, the Server examines its Connectors to determine how many threads are required. If its thread pool does not meet the Connectors' requirements, an IllegalStateException is thrown and the Server fails to start. However, JettyEmbeddedServletContainer temporarily removes the Server's Connectors while it is being started so that requests will not be accepted until the application is ready. This has the unwanted side-effect of causing a misconfigured thread pool to go undetected as the Connectors' thread requirements are not taken into consideration. The verification of the thread pool configuration and the starting of the Connectors is done in the Server's doStart() method which has three steps that are of interest: 1. Verify the Server's thread pool configuration 2. Start any managed beans that have been added to the Server 3. Start the Server's Connectors. To allow the thread pool configuration to be verified while still preventing the Connectors from being started, the Connectors need to be removed in step 2. This is achieved by registering a managed bean with the Server that nulls out the Server's Connectors as part of its doStart() method. Closes gh-8917 --- .../jetty/JettyEmbeddedServletContainer.java | 17 ++++++++++++++--- ...tyEmbeddedServletContainerFactoryTests.java | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java index 8941c3d614..d47ad9a7b7 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainer.java @@ -27,6 +27,7 @@ import org.eclipse.jetty.server.NetworkConnector; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.HandlerCollection; import org.eclipse.jetty.server.handler.HandlerWrapper; +import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.springframework.boot.context.embedded.EmbeddedServletContainer; import org.springframework.boot.context.embedded.EmbeddedServletContainerException; @@ -84,11 +85,21 @@ public class JettyEmbeddedServletContainer implements EmbeddedServletContainer { private void initialize() { synchronized (this.monitor) { try { - // Cache and clear the connectors to prevent requests being handled before - // the application context is ready + // Cache the connectors and then remove them to prevent requests being + // handled before the application context is ready. this.connectors = this.server.getConnectors(); - this.server.setConnectors(null); + this.server.addBean(new AbstractLifeCycle() { + @Override + protected void doStart() throws Exception { + for (Connector connector : JettyEmbeddedServletContainer.this.connectors) { + Assert.state(connector.isStopped(), "Connector " + connector + + " has been started prematurely"); + } + JettyEmbeddedServletContainer.this.server.setConnectors(null); + } + + }); // Start the server so that the ServletContext is available this.server.start(); this.server.setStopAtShutdown(false); diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java index eb10c83b9a..232fa5bad5 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java @@ -42,6 +42,7 @@ import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.server.handler.HandlerCollection; import org.eclipse.jetty.server.handler.HandlerWrapper; import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.util.thread.ThreadPool; import org.eclipse.jetty.webapp.Configuration; import org.eclipse.jetty.webapp.WebAppContext; @@ -59,6 +60,7 @@ import org.springframework.boot.web.servlet.ServletRegistrationBean; import org.springframework.http.HttpHeaders; import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.CoreMatchers.instanceOf; import static org.mockito.Matchers.anyObject; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; @@ -322,6 +324,22 @@ public class JettyEmbeddedServletContainerFactoryTests factory.getEmbeddedServletContainer().start(); } + @Test + public void startFailsWhenThreadPoolIsTooSmall() throws Exception { + JettyEmbeddedServletContainerFactory factory = getFactory(); + factory.addServerCustomizers(new JettyServerCustomizer() { + + @Override + public void customize(Server server) { + QueuedThreadPool threadPool = server.getBean(QueuedThreadPool.class); + threadPool.setMaxThreads(2); + threadPool.setMinThreads(2); + } + }); + this.thrown.expectCause(instanceOf(IllegalStateException.class)); + factory.getEmbeddedServletContainer().start(); + } + @Override @SuppressWarnings("serial") // Workaround for Jetty issue - https://bugs.eclipse.org/bugs/show_bug.cgi?id=470646