63

It is mentioned at multiple posts: improper use of ThreadLocal causes Memory Leak. I am struggling to understand how Memory Leak would happen using ThreadLocal.

The only scenario I have figured out it as below:

A web-server maintains a pool of Threads (e.g. for servlets). Those threads can create memory leak if the variables in ThreadLocal are not removed because Threads do not die.

This scenario does not mention "Perm Space" memory leak. Is that the only (major) use case of memory leak?

Sandeep Jindal
  • 14,510
  • 18
  • 83
  • 121
  • Can you clarify the question a little bit? Do you only want to know if/how a ThreadLocal's can cause PermGen exhaustion? – MRalwasser Jul 31 '13 at 13:44
  • 1
    Yes. I want to know - 1. How ThreadLocal cause PermGen exhaustion. 2. Any other common scenario of wrong usage of ThreadLocal causing memory leak. – Sandeep Jindal Jul 31 '13 at 13:46
  • 2
    There is a detailed explanation at http://java.jiderhamn.se/2012/01/29/classloader-leaks-iv-threadlocal-dangers-and-why-threadglobal-may-have-been-a-more-appropriate-name/ The author also proposes an interesting leak prevention library. – Pino Apr 28 '16 at 12:50

7 Answers7

92

PermGen exhaustions in combination with ThreadLocal are often caused by classloader leaks.

An example:
Imagine an application server which has a pool of worker threads.
They will be kept alive until application server termination.
A deployed web application uses a static ThreadLocal in one of its classes in order to store some thread-local data, an instance of another class (lets call it SomeClass) of the web application. This is done within the worker thread (e.g. this action originates from a HTTP request).

Important:
By definition, a reference to a ThreadLocal value is kept until the "owning" thread dies or if the ThreadLocal itself is no longer reachable.

If the web application fails to clear the reference to the ThreadLocal on shutdown, bad things will happen:
Because the worker thread will usually never die and the reference to the ThreadLocal is static, the ThreadLocal value still references the instance of SomeClass, a web application's class - even if the web application has been stopped!

As a consequence, the web application's classloader cannot be garbage collected, which means that all classes (and all static data) of the web application remain loaded (this affects the PermGen memory pool as well as the heap).
Every redeployment iteration of the web application will increase permgen (and heap) usage.

=> This is the permgen leak

One popular example of this kind of leak is this bug in log4j (fixed in the meanwhile).

MRalwasser
  • 15,605
  • 15
  • 101
  • 147
  • 3
    The (may be dumb) question is - Why Web Server do not try to Kill the Worked Threads for that application when the application is stopped? I would guess/assume that Web Server would create the worker thread specific for the application. If webserver do not kill those threads, even in case of no ThreadLocal, the threads would remain there. – Sandeep Jindal Jul 31 '13 at 20:16
  • 2
    Threads should never be killed, they should only be notified/interrupted in order to terminate gently by itself. Also, threads are expensive to create and often shared across multiple applications within the same container - but this is implementation-specific. However, some application servers drop all threads of a stopped web application (depending on the product/configuration), or renew its threads periodically in order to prevent such leaks. This is implementation specific, too. Have a look at the details for tomcat: http://tomcat.apache.org/tomcat-7.0-doc/config/executor.html – MRalwasser Jul 31 '13 at 22:26
  • @MRalwasser: Thnaks for nice explanation. It is clear why the treadlocal vairable will not be garbage collected, but why the classsloader and other classes also not be garbage collected? other classes dont have to do naything with the class that has the threadlocal variable – Victor Aug 29 '13 at 15:25
  • @Victor: The problem is that the value of the ThreadLocal will also not be garbage collected. Because the value is an instance of the shut down web application, its classloader and hence all other classes are not garbage collected, too. – MRalwasser Aug 30 '13 at 10:49
  • 2
    MRalwasser, perfectly explained! – 99Sono Nov 25 '15 at 07:45
37

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.

Neil Madden
  • 538
  • 5
  • 13
  • 1
    Sometimes people use a custom class in threadLocal value without even realizing that. For example it can happen when you use double brace initialization for your thread local value because double brace initialization will create an anonymous class. I created a fix for thread local that fix that class loader leak for your web apps but preserve non-blocking access to the thread local values: github.com/codesinthedark/ImprovedThreadLocal Now you can use your classes in ThreadLocal and you will not have memory leak on webapp redeployment – user1944408 Jul 26 '16 at 01:45
  • This was helpful - So is the answer we shouldnt code for this usecase or is there a way to prevent this sort of memory leaks. – jtkSource Dec 18 '16 at 08:09
  • `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`. Can someone explain this??? Why is there a difference? Is it due to the classloader? – jackycflau Jun 25 '19 at 07:17
  • 2
    @jackycflau Every class that is loaded by the JVM has a reference back to the ClassLoader that loaded it - `obj.getClass().getClassLoader()`. By default each ClassLoader has a reference to every class that it has loaded. So if you store a custom class in a ThreadLocal that was loaded by a custom ClassLoader then it will prevent the ClassLoader being garbage collected and all the classes it loaded. But classes from the standard library are loaded by the boot classloader, which is never garbage collected anyway. – Neil Madden Jun 26 '19 at 08:30
3

There is nothing inherently wrong with thread locals: They do not cause memory leaks. They are not slow. They are more local than their non-thread-local counterparts (i.e., they have better information hiding properties). They can be misused, of course, but so can most other programming tools…

Refer to this link by Joshua Bloch

kuporific
  • 10,053
  • 3
  • 42
  • 46
MayurB
  • 3,609
  • 2
  • 21
  • 36
1

The previous posts explain the problem but don't provide any solution. I found that there is no way to "clear" a ThreadLocal. In a container environment where I'm handling requests, I finally just called .remove() at the end of every request. I realize that could be problematic using container managed transactions.

Steve11235
  • 2,849
  • 1
  • 17
  • 18
0

Below code, the instance t in the for iteration can not be GC. This may be a example of ThreadLocal & Memory Leak

public class MemoryLeak {

    public static void main(String[] args) {
        new Thread(new Runnable() {
            @Override
            public void run() {
                for (int i = 0; i < 100000; i++) {
                    TestClass t = new TestClass(i);
                    t.printId();
                    t = null;
                }
            }
        }).start();
    }


    static class TestClass{
        private int id;
        private int[] arr;
        private ThreadLocal<TestClass> threadLocal;
        TestClass(int id){
            this.id = id;
            arr = new int[1000000];
            threadLocal = new ThreadLocal<>();
            threadLocal.set(this);
        }

        public void printId(){
            System.out.println(threadLocal.get().id);
        }
    }
}
Yanhui Zhou
  • 872
  • 6
  • 20
0

Here is an alternative to ThreadLocal that doesn't have the memory leak problem:

class BetterThreadLocal<A> {
  Map<Thread, A> map = Collections.synchronizedMap(new WeakHashMap());

  A get() {
    ret map.get(Thread.currentThread());
  }

  void set(A a) {
    if (a == null)
      map.remove(Thread.currentThread());
    else
      map.put(Thread.currentThread(), a);
  }
}

Note: There is a new memory leak scenario, but it is highly unlikely and can be avoided by following a simple guide line. The scenario is keeping a strong reference to a Thread object in a BetterThreadLocal.

I never keep strong references to threads anyways because you always want to allow the thread to be GC'd when its work is done... so there you go: a memory leak-free ThreadLocal.

Someone should benchmark this. I expect it to be about as fast as Java's ThreadLocal (both essentially do a weak hash map lookup, just one looks up the thread, the other the ThreadLocal).

Sample program in JavaX.

And a final note: My system (JavaX) also keeps track of all WeakHashMaps and cleans them up regularly, so the last super-unlikely hole is plugged (long-lived WeakHashMaps that are never queried, but still have stale entries).

Stefan Reich
  • 1,000
  • 9
  • 12
  • This will not work for question asked. You still have the same memory leak problem as Threads will never be collected in Java web servers as their life cycle is independent of web app. Thus preventing values to be collected. And if those values are instances of classes loaded by app classloader your app will never be released. – Talijanac May 26 '18 at 15:22
  • Do not do this, it will be far slower than JDK ThreadLocal, as threadlocals are stored in the thread object and thus do not require any synchronization. This implementation will be slow and as the @Talijanac says it does not solve the memory leak issue. – lifesoordinary Dec 19 '18 at 12:25
  • @Taljanac If you keep thread for too long, you wil have a problem with ThreadLocal too. – Stefan Reich Dec 19 '18 at 15:38
0

Memory leak is caused when ThreadLocal is always existing. If ThreadLocal object could be GC, it will not cause memory leak. Because the entry in ThreadLocalMap extends WeakReference, the entry will be GC after ThreadLocal object is GC.

Below code create a lot of ThreadLocal and it never Memory leak and the thread of main is always live.

// -XX:+PrintGCDetails -Xms100m -Xmx100m 
public class Test {

    public static long total = 1000000000;
    public static void main(String[] args) {
        for(long i = 0; i < total; i++) {
            // give GC some time
            if(i % 10000 == 0) {
                try {
                    Thread.sleep(100);
                } catch (InterruptedException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
            }
            ThreadLocal<Element> tl = new ThreadLocal<>();
            tl.set(new Element(i));
        }
    }
}

class Element {
    private long v;
    public Element(long v) {
        this.v = v;
    }
    public void finalize() {
        System.out.println(v);
    }
}