Memory leak: Thread objects not picked up by GC or is it a classloading issue?

Question

I created a GUI (Eclipse Wizard) where a user can log in into a database. I swapped the db-access into an external thread. The method startDBCheck() is called from the listeners when user inputs some text. Here's the code:

public class DBPage
    extends MyWizardPage
{
    public final class ConnectionCheckingThread
        extends Thread
    {
        boolean interrupted;

        @Override
        public void run()
        {
            updateStatus("Checking connection...");  // Method in MyWizardPage using Display.getDefault().asyncExec() to make changes to the GUI
            String dbError = Database.getInstance().checkDBAccess();  //checkDBAccess() is synchronized
            if (interrupted)
                return;
            if (dbError != null)
            {
                updateStatus("Error connecting database: " + dbError);
                return;
            }
            updateStatus(null);
        }

        public void disable()
        {
            interrupted = true;
        }
    }

    private ConnectionCheckingThread connectionCheckingThread;

    private void startDBCheck()
    {
        if (connectionCheckingThread != null)
            connectionCheckingThread.disable();
        connectionCheckingThread = new ConnectionCheckingThread();
        connectionCheckingThread.start();
    }
}

I am watching the memory usage and notice that the eclipse instance is taking more memory with every change on the graphical interface. If I play with the GUI for some time I get a OutOfMemoryError. This makes me think the dead threads aren't being deleted by garbage collector for some reason. I don't see any problem in my code though. If i set the reference on connectionCheckingThread to a new thread, there are no other references pointing the old one, so it should be removed. Am I right and the memory leak is somewhere else, or is there really a problem with this part of code?

upd: I get a PermGen space error which makes me consider if this code look suspicious.

private synchronized Connection getConnection()
    throws Exception
{
    // load JDBC drivers from jar file
    File driverFile = new File(pathToJar);
    URL[] urls = {driverFile.toURI().toURL()};
    URLClassLoader child = new URLClassLoader(urls, this.getClass().getClassLoader());

    Class< ? > clazz = Class.forName(className, true, child);
    if (clazz != null)
    {
        Driver driver = (Driver)clazz.newInstance();
        delegatingDriver.setDriver(driver);
        DriverManager.registerDriver(delegatingDriver);
    }

    // Open a connection. If no exception is thrown, the settings work
    return DriverManager.getConnection(dbURL, dbUser, dbPassword);
}

upd2: classloading is definitely the cause. Now I need to find out how to fix it.

Don't guess. Use a memory profiler (the JDK comes with jvisualvm), and see what consumes memory. — JB Nizet, Jul 23 '14 at 11:11
The Java garbage collector does not consider running threads to be garbage. That might be the cause of your problem. That would make this question a duplicate of http://stackoverflow.com/questions/2423284/java-thread-garbage-collected-or-not — Raedwald, Jul 23 '14 at 11:28
@Raedwald: but if I call `return` from the `run()`-method of a thread, it won't be running anymore, will it? — Danny Lo, Jul 23 '14 at 11:34
...if it gets as far as the `return`. If any of the methods called from your `run()` method block for a long time, the thread remains. — Raedwald, Jul 23 '14 at 11:36
Please look at this screenshot: http://i.imgur.com/SVHgpXU.png Would you agree if I say my threads (`connectionCheckingThread`s) finish properly? — Danny Lo, Jul 23 '14 at 11:42
Since I get a `PermGen space` error, maybe I should look for some classloading things...? — Danny Lo, Jul 23 '14 at 11:50
Do you load the `clazz` many times? Is it always the same? Make sure that it only results in one `Class` object (each load should have the same hashcode). Try saving the URLClassloader and "recycle" it. — Absurd-Mind, Jul 23 '14 at 12:12
Yeah, if you're creating new class loaders, you need to be VERY careful how they are managed or they (and the classes they loaded) will never go away. In the above code if DriverManager never goes away (or never clears the Driver reference) then everything else will be immortal. — Hot Licks, Jul 23 '14 at 12:22
And if you create a new class loader every time you create a new Driver, you will have N instances of URLClassLoader and N instances of the Driver Class object loaded, even if the Driver class is the exact same class every time. (In addition to your N instances of Driver objects.) — Hot Licks, Jul 23 '14 at 12:26
Is there a possibility to instantiate a classloader like this in a constructor: `childClassloader = new URLClassLoader(new URL[]{}, this.getClass().getClassLoader());` and to add URLs later on? I don't know how to "recycle" it otherways, because the URLs should be able to be changed. — Danny Lo, Jul 23 '14 at 12:28
Possible duplicate of http://stackoverflow.com/questions/20113926/why-does-permgen-outofmemeoryerror-occurs — Raedwald, Jul 23 '14 at 12:36
@HotLicks i see, but shouldn't it be collected by GC if there's no references to the instance? The class where the driver class is loaded is a singleton, which has only one instance of delegatingDriver. I keep a single instance of a loaded class only in the `delegatingDriver`. If the new reference is created, the old one is just replaced by the new one in the setter method. — Danny Lo, Jul 23 '14 at 12:48
All it takes is one reference somewhere to an instance loaded by that class loader to keep the class loader and all the classes it loaded from being collected. (And note too that some JVMs require a special setting to enable class unloading.) — Hot Licks, Jul 23 '14 at 14:58

score 0 · Accepted Answer · answered Jul 23 '14 at 14:20

I decided not to use DelegatingDriver as well as DriverManager anymore. Plus I created a subclass of a URLClassLoader to make it possible to add URLs dynamically. Here's my solution:

public class Database
{
    private static final class InstanceHolder
    {
        static final Database INSTANCE = new Database();
    }

    private static final class DynamicURLClassLoader
        extends URLClassLoader
    {

        public DynamicURLClassLoader(URL[] urls, ClassLoader parent)
        {
            super(urls, parent);
        }


        @Override
        protected void addURL(URL url)
        {
            super.addURL(url);
        }

    }

    // skipped

    private DynamicURLClassLoader childClassloader;

    private Database()
    {
        childClassloader = new DynamicURLClassLoader(new URL[]{}, this.getClass().getClassLoader());
    }

    public static Database getInstance()
    {
        return InstanceHolder.INSTANCE;
    }

    private synchronized Connection getConnection()
        throws Exception
    {
        // load JDBC drivers from jar file
        File driverFile = new File(pathToJar);
        childClassloader.addURL(driverFile.toURI().toURL());

        java.util.Properties info = new java.util.Properties();

        if (dbUser != null)
        {
            info.put("user", dbUser);
        }
        if (dbPassword != null)
        {
            info.put("password", dbPassword);
        }

        if (dbURL == null)
        {
            throw new SQLException("The url cannot be null", "08001");
        }
        Class< ? > clazz = Class.forName(className, true, childClassloader);
        if (clazz != null)
        {
            Driver driver = (Driver)clazz.newInstance();
            // Open a connection. If no exception is thrown, the settings work
            Connection con = driver.connect(dbURL, info);
            if (con != null)
                // Success!
                return (con);
        }
        throw new SQLException("The chosen driver is not suitable for " + dbURL, "08001");
    }
}

Memory leak: Thread objects not picked up by GC or is it a classloading issue?

1 Answers1