Bug #73876 Connector/J's AbandonedConnectionCleanupThread still uses WebappClassLoader

• Submitted: 10 Sep 2014 14:54
• Modified: 11 Sep 2014 7:39
• Reporter: Christopher Schultz
• Status: Duplicate
• Category: Connector / J
• Severity: S3 (Non-critical)
• Version: 5.1.32
• OS: Any
• Assigned to: Assigned Account
• CPU Architecture: Any
• Tags: memory leak

[10 Sep 2014 14:54] Christopher Schultz

Description:
Bug 65909 was fixed by adding a static AbandonedConnectionCleanupThread.shutdown() method to allow a server to kill the abandoned connection cleanup thread as the server is coming down. However, once stopped, that thread will not be restarted. If that method is called from ServletContextListener.contextDestroyed then web application reloads are not possible if the AbandonedConnectionCleanupThread is necessary.

Please see http://markmail.org/message/dmvlkps7lbgpngil for details.

How to repeat:
1. Deploy a web application using Connector/J
2. Re-deploy the web application
3. Use a profiler to see that the initial deployment's WebappClassLoader is pinned in memory by the AbandonedConnectionCleanupThread

Suggested fix:
Note that calling AbandonedConnectionCleanupThread.shutdown() is not a good fix, because the thread will never be restarted.

The proper solution to this situation is to set the ContextClassLoader for the AbandonedConnectionCleanupThread to be the ClassLoader that loaded the driver class itself (com.mysql.jdbc.NonRegisteringDriver).

Connector/J 5.1.25 removed the replacement of the cleanup thread's CCL with "null" (which is ineffective because you want the server's ClassLoader, not the application ClassLoader which is what you'll get if you use null). The technique of setting the CCL to null /did/ allow web applications to reload without shutting down the cleanup thread but probably didn't work properly because null is not appropriate.

Instead, you really want this in NonRegisteringDriver's static initializer:

  static {
    ClassLoader cl = Thread.currentThread().getContextClassLoader();

    try
    {
      Thread.currentThread().setContextClassLoader(NonRegisteringDriver.class.getClassLoader());

      AbandonedConnectionCleanupThread referenceThread = new AbandonedConnectionCleanupThread();
      referenceThread.setDaemon(true);
      referenceThread.start();
    }
    finally
    {
      Thread.currentThread.setContextClassLoader(cl);
    }
  }

Setting the thread's ContextClassLoader to that which loaded the driver means that the WebappClassLoader will only be used when the driver was loaded from WEB-INF/lib instead of a higher-level ClassLoader. This will avoid pinning the WebappClassLoader on restarts and does not require a call to AbandonedConnectionCleanupThread.shutdown() unless the driver is being loaded from WEB-INF/lib.

[10 Sep 2014 15:01] Christopher Schultz

Here is a formal patch that implements my suggestion:

--- src/com/mysql/jdbc/NonRegisteringDriver.java.orig	2014-09-10 10:55:31.000000000 -0400
+++ src/com/mysql/jdbc/NonRegisteringDriver.java	2014-09-10 10:59:44.000000000 -0400
@@ -112,9 +112,15 @@
 	
 	
 	static {
+		ClassLoader cl = Thread.currentThread().getContextClassLoader();
+		try {
+			Thread.currentThread().setContextClassLoader(NonRegisteringDriver.class.getClassLoader());
 		AbandonedConnectionCleanupThread referenceThread = new AbandonedConnectionCleanupThread();
 		referenceThread.setDaemon(true);
 		referenceThread.start();
+		} finally {
+			Thread.currentThread().setContextClassLoader(cl);
+		}
 	}
 	/**
 	 * Key used to retreive the database value from the properties instance

[10 Sep 2014 19:11] Christopher Schultz

Tweaked synopsis.

[11 Sep 2014 7:39] Alexander Soklakov

Hi Christopher,

Thanks for your comments. I mark this report as a duplicate of Bug#69526.