The accepted answer to this question, and the "severe" logs from Tomcat about this issue are misleading. The key quote there is:
By definition, a reference to a ThreadLocal value is kept until the "owning" thread dies or if the ThreadLocal itself is no longer reachable. [My emphasis].
In this case the only references to the ThreadLocal are in the static final field of a class that has now become a target for GC, and the reference from the worker threads. However, the references from the worker threads to the ThreadLocal are WeakReferences!
The values of a ThreadLocal are not weak references, however. So, if you have references in the values of a ThreadLocal to application classes, then these will maintain a reference to the ClassLoader and prevent GC. However, if your ThreadLocal values are just integers or strings or some other basic object type (e.g., a standard collection of the above), then there should not be a problem (they will only prevent GC of the boot/system classloader, which is never going to happen anyway).
It is still good practice to explicitly clean up a ThreadLocal when you are done with it, but in the case of the cited log4j bug the sky was definitely not falling (as you can see from the report, the value is an empty Hashtable).
Here is some code to demonstrate. First, we create a basic custom classloader implementation with no parent that prints to System.out on finalization:
import java.net.*;
public class CustomClassLoader extends URLClassLoader {
public CustomClassLoader(URL... urls) {
super(urls, null);
}
@Override
protected void finalize() {
System.out.println("*** CustomClassLoader finalized!");
}
}
We then define a driver application which creates a new instance of this class loader, uses it to load a class with a ThreadLocal and then remove the reference to the classloader allowing it to be GC'ed. Firstly, in the case where the ThreadLocal value is a reference to a class loaded by the custom classloader:
import java.net.*;
public class Main {
public static void main(String...args) throws Exception {
loadFoo();
while (true) {
System.gc();
Thread.sleep(1000);
}
}
private static void loadFoo() throws Exception {
CustomClassLoader cl = new CustomClassLoader(new URL("file:/tmp/"));
Class<?> clazz = cl.loadClass("Main$Foo");
clazz.newInstance();
cl = null;
}
public static class Foo {
private static final ThreadLocal<Foo> tl = new ThreadLocal<Foo>();
public Foo() {
tl.set(this);
System.out.println("ClassLoader: " + this.getClass().getClassLoader());
}
}
}
When we run this, we can see that the CustomClassLoader is indeed not garbage collected (as the thread local in the main thread has a reference to a Foo instance that was loaded by our custom classloader):
$ java Main
ClassLoader: CustomClassLoader@7a6d084b
However, when we change the ThreadLocal to instead contain a reference to a simple Integer rather than a Foo instance:
public static class Foo {
private static final ThreadLocal<Integer> tl = new ThreadLocal<Integer>();
public Foo() {
tl.set(42);
System.out.println("ClassLoader: " + this.getClass().getClassLoader());
}
}
Then we see that the custom classloader is now garbage collected (as the thread local on the main thread only has a reference to an integer loaded by the system classloader):
$ java Main
ClassLoader: CustomClassLoader@e76cbf7
*** CustomClassLoader finalized!
(The same is true with Hashtable). So in the case of log4j they didn't have a memory leak or any kind of bug. They were already clearing the Hashtable and this was sufficient to ensure GC of the classloader. IMO, the bug is in Tomcat, which indiscriminately logs these "SEVERE" errors on shutdown for all ThreadLocals that have not been explicitly .remove()d, regardless of whether they hold a strong reference to an application class or not. It seems that at least some developers are investing time and effort on "fixing" phantom memory leaks on the say-so of the sloppy Tomcat logs.