EmbeddedJmxTrans should not spawn ScheduledExecutors directly
nicusX opened this issue · 11 comments
EmbeddedJmxTrans.start() spawns ScheduledExecutors directly, using the JDK Executors static factory.
AFAIK this is discouraged as it may cause issues in J2EE containers.
At least SpringEmbeddedJmxTrans should delegate the creation of a TaskScheduler to Spring, and Spring may be configured to delegate thread creation to the container, when needed.
Hello @nicusX,
I am aware that "big" J2EE servers used to recommend to not use your own threads but they seem less vocal about this (the servlet specs are much more "open minded" now).
Did you face any specific problem?
jmxtrans-agent has been extensively used at CloudBees on Tomcat, Jetty, Glassfish, and Wildfly without any problem.
Cyrille
Extract of JSR 340 Java™ Servlet Specification Version 3.1
§15.2.2 Web Application Environment
...
Servlet containers that are part of a Java EE technology-compliant implementation are required to support this syntax. Consult the Java EE Specification for more details. This type of servlet container must support lookups of such objects and calls made to those objects when performed on a thread managed by the servlet container.
This type of servlet container should support this behavior when performed on threads created by the developer, but are not currently required to do so. Such a requirement will be added in the next version of this specification. Developers are cautioned that depending on this capability for application-created threads is not recommended, as it is non-portable.
We are currently experiencing problems on a Pivotal tc Server (based on Tomcat 7).
Apparently it is unable to shutdown threads started by JMXtrans.
I'm evaluating this library to be included in an application that have to be as much cross-container as possible. Having had problems in the past with "big" J2EE sever we prefer to control any external resource such as threads by Spring.
Apparently it is unable to shutdown threads started by JMXtrans.
Can you share with us the warning message?
Can you enable info logs on org.jmxtrans.embedded and serach for a stop message "EmbeddedJmxTrans stopped. Metrics have been collected and exported one last time.".
Cyrille
Setting org.jmxtrans.embedded logs to DEBUG I may see messages on application startup but none when the application shut down.
The container logs a WARN:
WARNING: The web application [sampleProject-dev] appears to have started a thread named [jmxtrans-collect-1] but has failed to stop it. This is very likely to create a memory leak. Stack trace of thread:
sun.misc.Unsafe.park(Native Method)
java.util.concurrent.locks.LockSupport.parkNanos(LockSupport.java:226)
java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.awaitNanos(AbstractQueuedSynchronizer.java:2082)
java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(ScheduledThreadPoolExecutor.java:1090)
java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(ScheduledThreadPoolExecutor.java:807)
java.util.concurrent.ThreadPoolExecutor.getTask(ThreadPoolExecutor.java:1068)
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
java.lang.Thread.run(Thread.java:745)
...and the Query is still running, as I may see the debug logging from org.jmxtrans.embedded.QueryAttribute
Apparently Spring is ignoring @PreDestroy, despite of the <context:annotation-config /> we have...
I have to investigate...
Strangely enough, it is NOT ignoring the @PostConstruct in the same class, as I may see
INFO o.j.e.spring.SpringEmbeddedJmxTrans - EmbeddedJmxTrans started
Surprising because SpringEmbeddedJmxTrans implements DisposableBean, it's not relying on annotations (that may be a bit more fragile).
I suspect it is not a matter of using annotations or interface for lifecycle hooks.
We are creating the SpringEmbeddedJmxTrans instance using EmbeddedJmxTransFactory:
<bean id="jmxTrans" class="org.jmxtrans.embedded.spring.EmbeddedJmxTransFactory" />
AFAIK beans created by FactoryBean doesn't have lifecycle hooks, as pointed here for example: http://spring.io/blog/2011/08/09/what-s-a-factorybean
Specifically in the last sentence:
One important takeaway here is that it is the FactoryBean, not the factoried object itself, that lives in the Spring container and enjoys the lifecycle hooks and container services. The returned instance is transient - Spring knows nothing about what you’ve returned from getObject() , and will make no attempt to exercise any lifecycle hooks or anything else on it.
The method EmbeddedJmxTrans.start() is executed as it is called explicitly by EmbeddedJmxTransFactory.getObject(), not for being annotated with @PostConstruct.
But EmbeddedJmxTrans.stop() will never been called.
Hi @nicusX, thanks for the investigations, can you test https://oss.sonatype.org/content/repositories/snapshots/org/jmxtrans/embedded/embedded-jmxtrans/1.0.12-SNAPSHOT/embedded-jmxtrans-1.0.12-20150115.112530-1.jar ?
Hi Cyrille
apparently now the the destroy() method is called.
11:14:35.810 [localhost-startStop-2] INFO o.j.e.spring.SpringEmbeddedJmxTrans - EmbeddedJmxTrans stopped. Metrics have been collected and exported one last time.
11:14:35.811 [localhost-startStop-2] DEBUG o.j.e.spring.SpringEmbeddedJmxTrans - ShutdownHook successfully removed
and Tomcat now doesn't warn about unstopped threads.
Thanks
Lorenzo
Thanks @nicusX!
embedded-jmxtrans 1.0.12 has been released, it will soon be available on Maven Central.
embedded-jmxtrans-1.0.12 is available on Maven Central Repository.
Please open a new bug if you still face issues.
Cyrille