Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JVM can't exit due to threads left if Tomcat throws exceptions during shutdown #16892

Closed

Conversation

jpmsilva
Copy link
Contributor

@jpmsilva jpmsilva commented May 17, 2019

Summary
If TomcatWebServer#rethrowDeferredStartupExceptions throws an exception, the embedded Tomcat instance may not allow the the JVM to terminate.

Symptoms
We have been running into some sporadic issues in which startup errors will sometime leave the JVM running, although the main thread has since died (the one running the @SpringBootApplication main).

Things like a failing Liquibase migration, that were being applied fairly early during the startup of the application, would leave the service hanging, and not truly failing (exiting with a status code != 0).

Analysis
The problem stems from the fact that some beans require early initialization in order for them to be injected into the Tomcat context.
Beans like Servlet Filters (and Servlet Filter Registrations) or Servlet Listeners (and Servlet Listener Registrations) are needed before the Tomcat context is started, so they (and their dependents) are created very early.
The remaining beans are created after the Tomcat context has been created.

If an exception occurs after the Tomcat context is started, the Tomcat is properly disposed of. However, if an error occurs early, the instance will not be fully created, and the instance will not be closed and destroyed.

This leaves some non daemon threads running and, those kind of threads do not allow the JVM to terminate, even if the exception propagates all the way to the main method.

Technical background
The method org.springframework.boot.web.embedded.tomcat.TomcatStarter#onStartup is called to run all ServletContextInitializers
Spring Boot uses a ServletContextInitializer to initialize the Spring framework that comes from org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext#getSelfInitializer.

This ServletContextInitializer calls org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext#getServletContextInitializerBeans to fetch available org.springframework.boot.web.servlet.ServletContextInitializerBeans

Back to org.springframework.boot.web.embedded.tomcat.TomcatWebServer#initialize, after tomcat.start() is done, org.springframework.boot.web.embedded.tomcat.TomcatWebServer#rethrowDeferredStartupExceptions is called to rethrow any deferred exceptions, which is immediately caught and:

Since initialize() is called from the constructor of TomcatWebServer, a webServer instance is never stored in org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext#createWebServer

Down the stack, org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext#refresh will catch the WebServerException and call org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext#stopAndReleaseWebServer.
However, since a webServer was never fully initialized, there is none to call stop() upon.

Notice that stop() is not the same as stopSilently(). stop() will stop and destroy and destroy the Tomcat instance. stopSilently() does not destroy the Tomcat instance.

Root cause
A Tomcat server (org.apache.catalina.Server) depends on destroy to release certain resources.
One such resource is the utilityExecutor stored in org.apache.catalina.core.StandardServer.
This executor creates threads that are not marked as daemon (org.apache.catalina.core.StandardServer#utilityThreadsAsDaemon is by default false).
Since these threads are not daemon, they do not allow the JVM to die after the main thread finishes.
The executor is stopped from org.apache.catalina.core.StandardServer#destroyInternal, which depends on Tomcat being destroyed.

If a Tomcat instance is not properly destroyed (not just closed), it may
leave non daemon threads running, which do not allow the JVM to exit.
@pivotal-issuemaster
Copy link

@jpmsilva Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 17, 2019
@pivotal-issuemaster
Copy link

@jpmsilva Thank you for signing the Contributor License Agreement!

@jpmsilva
Copy link
Contributor Author

I apologize beforehand for the denseness of the technical analysis, but I wanted to leaved well documented the rationale behind the fix, since the issue may be complicated to grasp, given that certain bean creation failures will trigger bug, while others will not.

@philwebb
Copy link
Member

apologize beforehand for the denseness of the technical analysis

@jpmsilva there's no need to apologize, the deep analysis is extremely useful!

@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels May 17, 2019
@philwebb philwebb added this to the 2.1.x milestone May 17, 2019
@philwebb philwebb changed the title Ensures Tomcat is destroyed when startup exceptions occur JVM can't exit due to threads left if Tomcat throws exceptions during shutdown May 28, 2019
philwebb added a commit that referenced this pull request May 28, 2019
* pr/16892:
  Polish "Allow Tomcat be destroyed regardless of exceptions"
  Allow Tomcat be destroyed regardless of exceptions
@philwebb philwebb modified the milestones: 2.1.x, 2.1.6 May 28, 2019
@philwebb philwebb closed this in 2b33e31 May 28, 2019
@philwebb
Copy link
Member

@jpmsilva Thanks for the PR. This has now been merged to 2.1.x and master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants