shutdown hook

[Hontvári József Levente]

I see that a shutdown hook is installed in the Metrics class. I had a
problem previously with the shutdown hook in EHCache (which is now
disabled by default).

The problem is that shutdown hooks are called in a random order. If
Metrics is used in an application which itself uses a shutdown hook,
than it is quite possible that the JmxReporter will be shut down before
the primary parts of the the application is shut down. Which may or
may not cause an issue. Likely not, the application itself rarely
depends on JMX beans.

[Hontvári József Levente]

Now that I use Metrics in production, the possible issue I mentioned in
my original email indeed does occur. Apache Tomcat uses a shutdown hook
itself, it calls my code, which calls Metrics.shutdown, which tries to
remove its own shutdown hook... and fails because the shutdown hooks are
already being called.
Metrics is a library, and not an application, therefore it does not know
about the environment in which it will run. It should not try to manage
its lifecycle itself. So the solution would be to use a system property
which controls whether the shutdown hook - as a convenience feature - is
installed or not.

2012-07-05 14:26:21Z ERROR ps.common.Webapp Graceful shutdown failed
java.lang.IllegalStateException: Shutdown in progress
at java.lang.ApplicationShutdownHooks.remove(Unknown Source)
at java.lang.Runtime.removeShutdownHook(Unknown Source)
at com.yammer.metrics.Metrics.shutdown(Metrics.java:342)
at ps.common.Webapp.destroyApplication(Webapp.java:317)
at ps.common.Webapp.contextDestroyed(Webapp.java:88)
at
org.apache.catalina.core.StandardContext.listenerStop(StandardContext.java:4011)

[Matt Brown]

Why is it necessary to unregister MBeans when the JVM process is shutting down anyway?

[Hontvári József Levente]

On 2012.09.21. 2:18, Matt Brown wrote:

Why is it necessary to unregister MBeans when the JVM process is shutting down anyway?

A web application specifies a listener, which receives notification about the shutdown of the WEB APPLICATION. There can be two reason for this: either the whole application server is shutting down or only this specific web application. The specific reason is unknown to the web application. The first case is the one you mention, the JVM is shutting down, in that case it is indeed not necessary to unregister MBeans. At least I don't know about any issues, except Tomcat maybe logs a warning. In the second case, when the webapp stops temporarily, before a restart, maybe combined with an upgrade, the MBean must be unregistered. Otherwise the result is at least a memory leak, or in the worse - and more frequent - case, the webapp will not be able to start again, because an MBean with the same name is already registered. MBeans are global resources (unfortunately).

[Matt Brown]

I assumed your original question referred to this code in the Metrics class:

    static {

        ...

        Runtime.getRuntime().addShutdownHook(SHUTDOWN_HOOK);

    }

Runtime shutdownHooks are only invoked when the JVM process shuts down, not when the web application context alone is stopped or reloaded. 

So unless I'm missing something it seems like shutting down the JMX reporter should be done in a ServletContextListener, like the already-existing MetricsServletContextListener does - which is why I am curious what the need would be to unregister MBeans in a non-web application if the JVM process is shutting down anyway.

[Hontvári József Levente]

On 2012.09.21. 15:58, Matt Brown wrote:

I assumed your original question referred to this code in the Metrics class:

    static {

        ...

        Runtime.getRuntime().addShutdownHook(SHUTDOWN_HOOK);

    }

yes, I referred to this code

Runtime shutdownHooks are only invoked when the JVM process shuts down, not when the web application context alone is stopped or reloaded. 

So unless I'm missing something it seems like shutting down the JMX reporter should be done in a ServletContextListener, like the already-existing MetricsServletContextListener does - which is why I am curious what the need would be to unregister MBeans in a non-web application if the JVM process is shutting down anyway.

sorry, I misunderstood your question. I do not see any benefit of the shutdown hook. I originally proposed that the shutdown hook installation should be optional and the default behavior should be to not install it. But after thinking about your question my opinion is that it should be completely removed. It feels wrong in a library, it in fact caused problems, and there is no case when it would be useful.