From 30de75c75ccbeb4efcd9222df4acb9952b858a89 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 3 Apr 2018 13:49:53 +0100 Subject: [PATCH 1/3] Ensure that Jetty is completely stopped when it fails to start Closes gh-12735 --- .../embedded/jetty/JettyEmbeddedServletContainer.java | 4 +++- .../JettyEmbeddedServletContainerFactoryTests.java | 11 ++++++++++- 2 files changed, 13 insertions(+), 2 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 d47ad9a7b7d..fa2d97b2994 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 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. @@ -155,9 +155,11 @@ public class JettyEmbeddedServletContainer implements EmbeddedServletContainer { .info("Jetty started on port(s) " + getActualPortsDescription()); } catch (EmbeddedServletContainerException ex) { + stopSilently(); throw ex; } catch (Exception ex) { + stopSilently(); throw new EmbeddedServletContainerException( "Unable to start embedded Jetty servlet container", ex); } 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 58ebaa52a3b..f78c965ac34 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 @@ -357,7 +357,16 @@ public class JettyEmbeddedServletContainerFactoryTests }); this.thrown.expect(EmbeddedServletContainerException.class); - factory.getEmbeddedServletContainer().start(); + JettyEmbeddedServletContainer jettyContainer = (JettyEmbeddedServletContainer) factory + .getEmbeddedServletContainer(); + try { + jettyContainer.start(); + } + finally { + QueuedThreadPool threadPool = (QueuedThreadPool) jettyContainer.getServer() + .getThreadPool(); + assertThat(threadPool.isRunning()).isFalse(); + } } @Test From edc00eef24c13b59f264e0be860fc8b9fd99bb52 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 3 Apr 2018 13:52:28 +0100 Subject: [PATCH 2/3] Ensure that Tomcat is completely stopped when its initialization fails Closes gh-12736 --- .../TomcatEmbeddedServletContainer.java | 1 + ...tEmbeddedServletContainerFactoryTests.java | 22 +++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java index 56af3dfe9cc..9b136eab2c8 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java @@ -134,6 +134,7 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer } } catch (Exception ex) { + stopSilently(); throw new EmbeddedServletContainerException( "Unable to start embedded Tomcat", ex); } diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java index d7ba095f3a2..5a0edda05f0 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java @@ -23,6 +23,7 @@ import java.util.Arrays; import java.util.Locale; import java.util.Map; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import javax.naming.InitialContext; import javax.naming.NamingException; @@ -491,7 +492,18 @@ public class TomcatEmbeddedServletContainerFactoryTests @Test public void faultyFilterCausesStartFailure() throws Exception { - AbstractEmbeddedServletContainerFactory factory = getFactory(); + final AtomicReference tomcatReference = new AtomicReference(); + TomcatEmbeddedServletContainerFactory factory = new TomcatEmbeddedServletContainerFactory( + 0) { + + @Override + protected TomcatEmbeddedServletContainer getTomcatEmbeddedServletContainer( + Tomcat tomcat) { + tomcatReference.set(tomcat); + return super.getTomcatEmbeddedServletContainer(tomcat); + } + + }; factory.addInitializers(new ServletContextInitializer() { @Override @@ -518,7 +530,13 @@ public class TomcatEmbeddedServletContainerFactoryTests }); this.thrown.expect(EmbeddedServletContainerException.class); - factory.getEmbeddedServletContainer().start(); + try { + factory.getEmbeddedServletContainer(); + } + finally { + assertThat(tomcatReference.get().getServer().getState()) + .isEqualTo(LifecycleState.STOPPED); + } } @Override From dee8750aff8dc3bb477bca8faea562d38a777183 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 3 Apr 2018 15:45:08 +0100 Subject: [PATCH 3/3] Stop Jetty in Jetty8JettyEmbeddedServletContainerFactoryTests Closes gh-12734 --- ...yEmbeddedServletContainerFactoryTests.java | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/Jetty8JettyEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/Jetty8JettyEmbeddedServletContainerFactoryTests.java index a4b411d5186..94840dd80ea 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/Jetty8JettyEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/Jetty8JettyEmbeddedServletContainerFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 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. @@ -70,23 +70,28 @@ public class Jetty8JettyEmbeddedServletContainerFactoryTests { }); jetty.start(); - int port = jetty.getPort(); - RestTemplate restTemplate = new RestTemplate(); - restTemplate.setErrorHandler(new ResponseErrorHandler() { - - @Override - public boolean hasError(ClientHttpResponse response) throws IOException { - return false; - } - - @Override - public void handleError(ClientHttpResponse response) throws IOException { - } - - }); - ResponseEntity response = restTemplate - .getForEntity("http://localhost:" + port, String.class); - assertThat(response.getBody()).isEqualTo("An error occurred"); + try { + int port = jetty.getPort(); + RestTemplate restTemplate = new RestTemplate(); + restTemplate.setErrorHandler(new ResponseErrorHandler() { + + @Override + public boolean hasError(ClientHttpResponse response) throws IOException { + return false; + } + + @Override + public void handleError(ClientHttpResponse response) throws IOException { + } + + }); + ResponseEntity response = restTemplate + .getForEntity("http://localhost:" + port, String.class); + assertThat(response.getBody()).isEqualTo("An error occurred"); + } + finally { + jetty.stop(); + } } private static final class TestServlet extends HttpServlet {