From d7d2404c23897e4747a70320931e1ca3083d1d87 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 8 Mar 2016 13:50:16 +0000 Subject: [PATCH] Ensure that Tomcat does not report unstopped main thread when startup fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following the changes made in ff99bb07, Tomcat’s WebAppClassLoader is set as the thread context class loader of the main thread. If Tomcat was stopped while the main thread was still running (typically as a result of a startup failure), this had the unwanted side-effect of causing Tomcat to report that the application had started a thread named main and had failed to stop it. This commit updates TomcatEmbeddedServletContainer so that, during stop processing, the current thread’s context class loader is reset before Tomcat is stopped. This prevents Tomcat from incorrectly believe that the application has started and failed to stop the main thread. Closes gh-5357 --- .../tomcat/TomcatEmbeddedServletContainer.java | 17 +++++++++++------ ...catEmbeddedServletContainerFactoryTests.java | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 6 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 91ef1ed4e9..90810cb4a9 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 @@ -155,6 +155,7 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer } }; + awaitThread.setContextClassLoader(getClass().getClassLoader()); awaitThread.setDaemon(false); awaitThread.start(); } @@ -191,13 +192,21 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer private void stopSilently() { try { - this.tomcat.stop(); + stopTomcat(); } catch (LifecycleException ex) { // Ignore } } + private void stopTomcat() throws LifecycleException { + if (Thread.currentThread() + .getContextClassLoader() instanceof TomcatEmbeddedWebappClassLoader) { + Thread.currentThread().setContextClassLoader(getClass().getClassLoader()); + } + this.tomcat.stop(); + } + private void addPreviouslyRemovedConnectors() { Service[] services = this.tomcat.getServer().findServices(); for (Service service : services) { @@ -246,7 +255,7 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer public synchronized void stop() throws EmbeddedServletContainerException { try { try { - this.tomcat.stop(); + stopTomcat(); this.tomcat.destroy(); } catch (LifecycleException ex) { @@ -258,10 +267,6 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer ex); } finally { - if (Thread.currentThread() - .getContextClassLoader() instanceof TomcatEmbeddedWebappClassLoader) { - Thread.currentThread().setContextClassLoader(getClass().getClassLoader()); - } containerCounter.decrementAndGet(); } } 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 bbc0c1aa6a..4c160715c1 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 @@ -37,6 +37,7 @@ import org.apache.catalina.startup.Tomcat; import org.apache.catalina.valves.RemoteIpValve; import org.apache.coyote.http11.AbstractHttp11JsseProtocol; import org.junit.After; +import org.junit.Rule; import org.junit.Test; import org.mockito.InOrder; @@ -44,6 +45,7 @@ import org.springframework.boot.context.embedded.AbstractEmbeddedServletContaine import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactoryTests; import org.springframework.boot.context.embedded.EmbeddedServletContainerException; import org.springframework.boot.context.embedded.Ssl; +import org.springframework.boot.testutil.OutputCapture; import org.springframework.test.util.ReflectionTestUtils; import org.springframework.util.SocketUtils; @@ -67,6 +69,9 @@ import static org.mockito.Mockito.verify; public class TomcatEmbeddedServletContainerFactoryTests extends AbstractEmbeddedServletContainerFactoryTests { + @Rule + public OutputCapture outputCapture = new OutputCapture(); + @Override protected TomcatEmbeddedServletContainerFactory getFactory() { return new TomcatEmbeddedServletContainerFactory(0); @@ -335,6 +340,16 @@ public class TomcatEmbeddedServletContainerFactoryTests } + @Test + public void startupFailureDoesNotResultInUnstoppedThreadsBeingReported() + throws IOException { + super.portClashOfPrimaryConnectorResultsInPortInUseException(); + + String string = this.outputCapture.toString(); + assertThat(string) + .doesNotContain("appears to have started a thread named [main]"); + } + @Override protected void addConnector(int port, AbstractEmbeddedServletContainerFactory factory) {