-
Notifications
You must be signed in to change notification settings - Fork 40.2k
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
Default inclusion of AprLifecycleListener, configured APR connector and spring-boot-starter-actuator crashes the JVM at shutdown time #28472
Comments
Thanks for the detailed analysis. My initial feeling is that the best place to address this would be in Tomcat itself. While we could make some changes in Spring Boot, creating a synthetic If, as you have suggested, |
You could reuse the code from
Even if we (Tomat committers) implement the ref counter, it only solves the symptom, abuse of the listener at context-level which is conceptionally wrong. |
That would be a chunk of fairly complex code that I would prefer to have abstracted away. It would also be a Spring Boot-specific fix to what I still believe to be a more general problem.
We can certainly move it up to the |
That's true and I don't like that either. Though, the listener with a synthetical lifecylce sounds acceptable if managed by Spring Boot directly. I don't see a huge problem implementing this. Note that Netty uses a fork of Tomcat Native as well.
As described, moving to the I will try to work out a doc patch for the listener in the first place. |
Here's a more concise workaround: package com.example.demo;
import java.util.Iterator;
import org.apache.catalina.LifecycleListener;
import org.apache.catalina.core.AprLifecycleListener;
import org.springframework.boot.actuate.autoconfigure.web.ManagementContextConfiguration;
import org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactory;
import org.springframework.boot.web.server.WebServerFactoryCustomizer;
import org.springframework.context.annotation.Bean;
@ManagementContextConfiguration(proxyBeanMethods = false)
class ManagementWebServerCustomizerConfiguration {
@Bean
WebServerFactoryCustomizer<TomcatServletWebServerFactory> tomcatCustomizer() {
return (tomcat) -> {
Iterator<LifecycleListener> listeners = tomcat.getContextLifecycleListeners().iterator();
while (listeners.hasNext()) {
if (listeners.next() instanceof AprLifecycleListener) {
listeners.remove();
}
}
};
}
} This
|
@wilkinsona I have tried that and it gets invoked, but it is invoked twice. Once for the main context and once for the management context. I expected that I modify both contexts separately: Is there no straight forward way to modify both separately? |
I omitted something: it needs to go in a package that isn't covered by component scanning. Typically that means in a package that isn't the same or a sub-package of your |
Changed, I can confirm that it is now only called on the management Tomcat, though it is still an instance of the custom Tomcat factory. Maybe this is normal. |
It is normal. That's how we know which type of web server (Jetty, Netty, Tomcat, Undertow or a custom implementation) to start for the management context. |
…to avoid JVM crashes This basically document cases to avoid issues like spring-projects/spring-boot#28472
@wilkinsona Please have a look: apache/tomcat#456 |
A reference-counter would work, because it would prevent the first |
Correct, that is what I have proposed as a counter measure on Tomcat side. |
…to avoid JVM crashes This basically documents how to avoid issues like spring-projects/spring-boot#28472
FWIW, I have replicated the restrictions for many listeners from the website docs to Javadoc: apache/tomcat@9a5c21c |
This basically documents how to avoid issues like spring-projects/spring-boot#28472
This basically documents how to avoid issues like spring-projects/spring-boot#28472
This basically documents how to avoid issues like spring-projects/spring-boot#28472
This basically documents how to avoid issues like spring-projects/spring-boot#28472
@michael-o Do you have a link to this proposal please? |
The discussion basically happened here: apache/tomcat#456 |
It's largely due to the general concept of a |
I see, maybe this can be addressed in next major. I am certain that other containers offer the same as Tomcat does. This would also reduce friction, config and memory requirements. |
We've looked at it in the past but it was surprisingly complex. It may be worth revisiting at some point though. |
Thank you for the reference. Makes sense now, other containers don't support it straight away. |
This doesn't seem to be a problem that's affecting many Spring Boot users and, re-reading apache/tomcat#456, the consensus seems to be that this should be addressed by referencing count in Tomcat, thereby making it more robust for everyone not just users of Spring Boot. |
I am using (in a minimal test application):
PATH
/LD_LIBRARY_PATH
.Disclaimer: I am completely new to Spring Boot (not Spring), but am a Tomcat comitter, so forgive me lack of Spring Boot knowledge. My previous Spring apps are hosted on a vanilla Tomcat instances. This crash is a side effect of: #10079
The following can be observed when the application (JVM) is shut down with Ctrl+C/
SIGTERM
:FreeBSD:
IMPORTANT: This is neither a bug in APR, nor in Tomcat Native or Java. This is a bug in Spring Boot.
Cause of crash:
I have debugged this. To host the main context and the management context Spring Boot starts two
Tomcat
instances, thus twoServer
objects. Each instance serves one context. We are still in one JVM (OS process). For some strange reason theAprLifecycleListener
is added to the context lifecycle listeners, although the vanilla Tomcat distribution uses it strictly in theServer
only. It is imperative to understand how this listener works to understand this failure. Native libraries can only be loaded once and only once into the JVM process, having this listener by default in both contexts is already wrong. When vanilla Tomcat is shut down, it terminates APR globally by destroying the global APR pool then the JVM shuts down. What happens in Spring Boot?SIGTERM
, Tomcat instances are shut down in reverse orderSIGSEGV
/ACCESS_VIOLATION
because the global APR pool is already gone. Use after free, basically.Hence, here is the misconception in ef4cf4c: Since Tomcat is not the owner of the JVM, but Spring Boot is, Spring Boot must take care of it just like vanilla Tomcat does only once.
What should now be done:
AprLifecylceListener
from the context listTomcatServletWebServerFactory#addServerLifecycleListeners(LifecycleListener... contextLifecycleListeners)
it won't solve the issue either.BEFORE_INIT_EVENT
/AFTER_DESTROY_EVENT
, start the listener manually before all Tomcat instances are started, shut it down after all instances have been destroyed.What could be done in Tomcat to avoid such situations:
Lifecycle
objects ofServer
interfaceinit()
/destroy()
and perform init on 0 and destroy on 0 only. For instance, Jansi maintains such a counter:install()
/uninstall()
@markt-asf please add your opinion on the Tomcat side!
What I have tried to solve this (merely a workaround to proof that it can work):
factory.setContextLifecycleListeners(Collections.emptyList());
to set the same of the management Tomcat instance so I patched Spring Boot 2.5.x and replaced the JARs in the local Maven repo:
main()
method:Here is the result now:
I believe this kind of change like mine can be done w/o breaking current setups. I'd be happy to discuss a proper solution.
The text was updated successfully, but these errors were encountered: