MySQL Bugs: #68556: Tomcat can't stop a cleanup thread by clearReferencesStopThreads.

Bug #68556	Tomcat can't stop a cleanup thread by clearReferencesStopThreads.
Submitted:	4 Mar 2013 7:29	Modified:	15 Apr 2013 0:28
Reporter:	Sadao Hiratsuka	Email Updates:
Status:	Closed	Impact on me:	None
Category:	Connector / J	Severity:	S3 (Non-critical)
Version:	5.1.23	OS:	Any
Assigned to:	Alexander Soklakov	CPU Architecture:	Any
Tags:	regression

Description:
Tomcat can't stop an Abandoned connection cleanup thread
by clearReferencesStopThreads.

I think it is regression of Bug#65909.
Here is a sample context.xml

<Context clearReferencesStopThreads="true">
    <!-- Default set of monitored resources -->
    <WatchedResource>WEB-INF/web.xml</WatchedResource>
    <!-- Uncomment this to disable session persistence across Tomcat restarts -->
    <!--
    <Manager pathname="" />
    -->
    <!-- Uncomment this to enable Comet connection tacking (provides events
         on session expiration as well as webapp lifecycle) -->
    <!--
    <Valve className="org.apache.catalina.valves.CometConnectionManagerValve" />
    -->
</Context>

How to repeat:
Tomcat 6.0.36
MySQL Connector/J 5.1.22 and 5.1.23

(1) Connector/J 5.1.22

(1-1) clearReferencesStopThreads="true" in context.xml
-> logged 'unregister failed'
org.apache.catalina.loader.WebappClassLoader clearReferencesJdbc
SEVERE: The web application [/examples] registered the JDBC driver
[com.mysql.jdbc.Driver] but failed to unregister it when the web
application was stopped. To prevent a memory leak,
the JDBC Driver has been forcibly unregistered.

-> logged 'very likely to create a memory leak'
org.apache.catalina.loader.WebappClassLoader clearReferencesThreads
SEVERE: The web application [/examples] appears to have started
a thread named [Abandoned connection cleanup thread] but has
failed to stop it. This is very likely to create a memory leak.

-> after logged, The Abandoned connection cleanup thread is killed.

(2) Connector/J 5.1.23

(2-1) clearReferencesStopThreads="true" in context.xml
-> logged 'unregister failed'
-> *NOT* logged 'very likely to create a memory leak'
-> after logged, The Abandoned connection cleanup thread is *NOT* killed.

(2-2) When we implement ServletContextListener ...
-> logged 'unregister failed'
-> not logged 'very likely to create a memory leak'
-> The Abandoned connection cleanup thread is stopped properly.

Suggested fix:
--- com/mysql/jdbc/AbandonedConnectionCleanupThread.java_org    2013-03-04 16:23:26.595493300 +0900
+++ com/mysql/jdbc/AbandonedConnectionCleanupThread.java        2013-03-04 16:23:33.775171300 +0900
@@ -35,7 +35,6 @@

        public void run() {
                threadRef = this;
-               threadRef.setContextClassLoader(null);
                while (running) {
                        try {
                                Reference<? extends ConnectionImpl> ref = NonRegisteringDriver.refQueue.remove(100);

==

(3) Connector/J 5.1.23 with this patch

(3-1) clearReferencesStopThreads="true" in context.xml
-> logged 'unregister failed'
-> logged 'very likely to create a memory leak'
-> after logged, The Abandoned connection cleanup thread is killed.

(3-2) When we implement ServletContextListener ...
-> logged 'unregister failed'
-> not logged 'very likely to create a memory leak'
-> The Abandoned connection cleanup thread is stopped properly.

I think (3-1)'s behavior is better than (2-1).

Hi Sadao,

Thanks for wonderful catch! 3-1 is really much better, we should fix it.

Added changelog entry for 5.1.25:

"Tomcat would fail to stop an abandoned connection cleanup thread."

Please advise if this entry requires revision.

The 5.1.25 changelog entry has been revised as follows:

"Tomcat would fail to stop abandoned connection cleanup threads. The fix
for Bug#65909 introduced the ability to stop daemon threads started by the
Connector/J driver but it also cleared references from daemon threads to
the parent classloader. When the "clearReferencesStopThreads" property is
set to "true" in "context.xml", Tomcat analyzes classloaders to detect and
stop lost threads. This fix ensures that abandoned connection cleanup
threads retain a reference to the parent classloader."

This is a terrible fix for this bug, for a number of reasons.

First, by retaining the current contextClassLoader, you bind the thread to the WebappClassLoader and then need to resort to the kinds of hacks provided by Tomcat to fix the problem. It would be much better to fix the problem at its root.

Second, when Tomcat kills the AbandonedConnectionCleanupThread, it will never run again, so reloading a webapp will effectively kill this thread for the life of the JVM (for a standard Tomcat configuration where Connector/J is actually loaded from the container's ClassLoader and not the WebappClassLoader).

Third, the thread is launched from a static initializer in NonRegisteringDriver, like this:

        static {
                AbandonedConnectionCleanupThread referenceThread = new AbandonedConnectionCleanupThread();
                referenceThread.setDaemon(true);
                referenceThread.start();
        }

This means that there are no references to the thread in order to stop it nicely.

A better fix would be to:

1. Use a (private, probably) static member reference in NonRegisteringDriver to retain a reference to the thread.
2. Provide a static method in NonRegisteringDriver to shut down the thread.
3. Re-start the thread whenever appropriate.
4. Set the contextClassLoader of the thread *that creates the AbandonedConnectionCleanupThread* to be the ClassLoader that loaded NonRegisteringDriver *before* creating the AbandonedConnectionCleanupThread.

That last bit is fairly important, as it fixes a related leak that can't be prevented in another way: where Thread.inheritedAccessControlContext has references to the WebappClassLoader that are inherited from the context ClassLoader when the Thread is created (before any Connector/J code runs for the thread).

Please re-examine this bugfix and consider my suggestions. Otherwise, Connector/J will either continue to leak ClassLoaders on web application reloads, or this thread will be terminated on the first reload and never start again, significantly limiting its usefulness.

Here is a much better workaround until something else changes.

Enable Tomcat's JreMemoryLeakPreventionListener (enabled by default on Tomcat 7), and add this attribute to the <Listener> element:

  classesToInitialize="com.mysql.jdbc.NonRegisteringDriver"

If "classesToInitialize" is already set on your <Listener>, just add NonRegisteringDriver to the existing value separated by a comma.

I did some testing with the  JreMemoryLeakPreventionListener / classesToInitialize workaround (Tomcat 7.0.39 + MySQL Connector/J 5.1.25).

Before applying the workaround thread dumps listed multiple AbandonedConnectionCleanupThread instances after redeploying the webapp several times. After applying the workaround there's only one AbandonedConnectionCleanupThread instance.

I had to modify my app, though, and move MySQL driver from the webapp to Tomcat lib.
Otherwise, the classloader is unable to load com.mysql.jdbc.NonRegisteringDriver at Tomcat startup.