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 1/2] 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 9b98ecf470..2540914077 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 e3b8ed4227..2c427d4eaa 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 7403574e5c..d6ce1b7277 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() { From 09373622ca9323d1d35756f4ba542798237c9aac Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 28 May 2019 16:15:26 -0700 Subject: [PATCH 2/2] Polish "Allow Tomcat be destroyed regardless of exceptions" See gh-16892 --- .../boot/web/embedded/tomcat/TomcatWebServer.java | 2 +- .../web/embedded/tomcat/TomcatServletWebServerFactoryTests.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) 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 2540914077..d9d699235b 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 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. 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 2c427d4eaa..f33704a1b3 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 @@ -526,6 +526,7 @@ public class TomcatServletWebServerFactoryTests @Test public void exceptionThrownOnContextListenerDestroysServer() { TomcatServletWebServerFactory factory = new TomcatServletWebServerFactory(0) { + @Override protected TomcatWebServer getTomcatWebServer(Tomcat tomcat) { try { @@ -536,6 +537,7 @@ public class TomcatServletWebServerFactoryTests .isEqualTo(LifecycleState.DESTROYED); } } + }; assertThatExceptionOfType(WebServerException.class) .isThrownBy(() -> factory.getWebServer((context) -> context