From 2b33e31a7ccc3f3a44201b669a4dfd65ee155703 Mon Sep 17 00:00:00 2001 From: Joao Silva <30354367+jpmsilva@users.noreply.github.com> Date: Fri, 17 May 2019 13:31:40 +0100 Subject: [PATCH] Allow Tomcat be destroyed regardless of exceptions Update `TomcatWebServer` so that lifecycle exceptions are silently swallowed when attempting shutdown. Prior to this commit it was possible that a Tomcat instance might not be properly destroyed and could leave non daemon threads running, which prevent the JVM from exiting. Fixes gh-16892 --- .../web/embedded/tomcat/TomcatWebServer.java | 10 ++++++++++ .../TomcatServletWebServerFactoryTests.java | 19 +++++++++++++++++++ .../AbstractServletWebServerFactoryTests.java | 9 +++++++++ 3 files changed, 38 insertions(+) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java index 9b98ecf4707..25409140770 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java @@ -122,6 +122,7 @@ public class TomcatWebServer implements WebServer { } catch (Exception ex) { stopSilently(); + destroySilently(); throw new WebServerException("Unable to start embedded Tomcat", ex); } } @@ -242,6 +243,15 @@ public class TomcatWebServer implements WebServer { } } + private void destroySilently() { + try { + this.tomcat.destroy(); + } + catch (LifecycleException ex) { + // Ignore + } + } + private void stopTomcat() throws LifecycleException { if (Thread.currentThread() .getContextClassLoader() instanceof TomcatEmbeddedWebappClassLoader) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java index e3b8ed42275..2c427d4eaa5 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java @@ -523,6 +523,25 @@ public class TomcatServletWebServerFactoryTests assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); } + @Test + public void exceptionThrownOnContextListenerDestroysServer() { + TomcatServletWebServerFactory factory = new TomcatServletWebServerFactory(0) { + @Override + protected TomcatWebServer getTomcatWebServer(Tomcat tomcat) { + try { + return super.getTomcatWebServer(tomcat); + } + finally { + assertThat(tomcat.getServer().getState()) + .isEqualTo(LifecycleState.DESTROYED); + } + } + }; + assertThatExceptionOfType(WebServerException.class) + .isThrownBy(() -> factory.getWebServer((context) -> context + .addListener(new FailingServletContextListener()))); + } + @Override protected JspServlet getJspServlet() throws ServletException { Tomcat tomcat = ((TomcatWebServer) this.webServer).getTomcat(); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java index 7403574e5cb..d6ce1b72779 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java @@ -1401,6 +1401,15 @@ public abstract class AbstractServletWebServerFactoryTests { } + public static class FailingServletContextListener implements ServletContextListener { + + @Override + public void contextInitialized(ServletContextEvent sce) { + throw new FailingServletException(); + } + + } + private static class FailingServletException extends RuntimeException { FailingServletException() {