Threadlocal remove?

Question

When using a ThreadLocal should I always call remove() when I am done or when I do set the old value is replaced anyway so remove is redundant?

Solution 1

Because ThreadLocal has Map of currentThread and value, Now if you don't remove the value in the thread which was using it then it will create a memory leak.

You should always call remove because ThreadLocal class puts values from the Thread Class defined by ThreadLocal.Values localValues; This will also cause to hold reference of Thread and associated objects.

From the source code of ThreadLocal

the value will be set to null and the underlying entry will still be present.

Solution 2

set always replaces the old value.

This is true for

• Calendar.set() and Date.set()
• BitSet.set()
• List.set()
• setters

You mean without remove it will not be GCed?

It will not be removed until the thread dies. It won't disappear on you without you calling remove()

Whether this is a memory leak or not depends on your program. You would have to create lots of threads with large thread local objects which you didn't need for some reason for it to matter. e.g. 1000 threads with a 1 KB object could waste up to 1 MB, but this suggest a design issue if you are doing this sort of thing.

---

The only place you might get a memory leak is.

for (int i = 0; ; i++) {
    // don't subclass Thread.
    new Thread() {
        // this is somewhat pointless as you are defining a ThreadLocal per thread.
        final ThreadLocal<Object> tlObject = new ThreadLocal<Object>() {
        };

        public void run() {
            tlObject.set(new byte[8 * 1024 * 1024]);
        }
    }.start();
    Thread.sleep(1);
    if (i % 1000 == 0) {
        System.gc();
        System.out.println(i);
    }
}

with -verbosegc prints.

[Full GC 213548K->49484K(3832192K), 0.0334194 secs]
39000
[GC 2786060K->82412K(3836864K), 0.0132035 secs]
[GC 2815569K->107052K(3836544K), 0.0212252 secs]
[GC 2836162K->131628K(3837824K), 0.0199268 secs]
[GC 2867613K->156204K(3837568K), 0.0209828 secs]
[GC 2886894K->180780K(3838272K), 0.0191244 secs]
[GC 2911942K->205356K(3838080K), 0.0187482 secs]
[GC 421535K->229932K(3838208K), 0.0192605 secs]
[Full GC 229932K->49484K(3838208K), 0.0344509 secs]
40000

Note: the size after a full GC is the same 49484K

In the above case you will have a ThreadLocal which refers to the Thread which refers to the ThreadLocal. However, as the Thread is dead it doesn't cause a memory leak becasue it becomes a regard object i.e. when A -> B and B -> A

I ran the above example in a loop for a couple of minutes and the GC levels moved around alot but the minimum size was still small.

Solution 3

No you do not have to "always call remove()" instead of set()

If you fear memory leaks doing so, here is what the javadoc says

Each thread holds an implicit reference to its copy of a thread-local variable as long as the thread is alive and the ThreadLocal instance is accessible; after a thread goes away, all of its copies of thread-local instances are subject to garbage collection (unless other references to these copies exist).

Thus not calling remove() will not prevent a thread-local instance to be properly garbage collected, and will not cause memory leaks by nature.

You can also take a look to ThreadLocal implementation, which use WeakReferences for this "implicit reference" mechanism

But beware of consistency with thread pools

Using set() method only with a thread pool, you might prefer to remove() a ThreadLocal instance, instead of overriding it in another "work unit" using the same thread. Because you might want to avoid the situation where, for some reason, the set method is not invoked, and your ThreadLocal remains attached to a context/treatment it does not belong.

Solution 4

set: Sets the current thread's copy of this thread-local variable to the specified value.

Meaning whatever was in that memory location, will now be overwritten by what you passed through set

Solution 5

If the variable you're trying to remove will be always set in next executions of the thread, I wouldn't worry about removing it. set will overwrite its value.

But if you're setting that variable only in some circusmtances (for instance, when treating only a specific kind of requests), removing it might be convenient so that it doesn't stay around when, for instance, the thread is put back into the pool.

View more solutions

Comments

[Jim]

You mean without remove it will not be GCed?

[Amit Deshpande]

It is not about value it is about Thread which has reference in ThreadLocal if you don't call remove it will never get GCed.

[Vishy]

@Jim Good question, have added an answer.

[bestsss]

Peter, leak can happen on extended ThreadLocals since the thread is going to keep a reference to. Think of undeploying and dynamic classloaders.

[Vishy]

The ThreadLocal doesn't actually hold the object. The object is held in a ThreadLocalMap in the Thread itself where the ThreadLocal object is the key. When the Thread stops, all its thread local objects can become unreachable (unless they are held elsewhere)

[Vishy]

ThreadLocal is not final but the only time I have seen it subclassed is to define an initialValue. If this is defined in the Thread's context you have a problem, but sub-classing Thread is generally considered a bad idea.

[bestsss]

Yeah, initalValue is the culprit usually but not necessarily. The point is that threads can be orthogonal and if they are not: calling services or responding to RMI would create a instance in the Thread map. I'd say left/forgotten ThreadLocals are #1 offender when it comes to class loader leaks.

[Vishy]

It tried constructing a sub-classed ThreadLocal with a reference to the Thread itself. But I couldn't produce a memory leak.

[Vishy]

@bestsss I will have to take your word for it :)

[Jim]

I don't understand your differentiation of set(null) and remove. Should I call set(null) before calling set?Why would this cause a leak?

[irreputable]

@bestsss interesting distinction.

[bestsss]

The leak doesn't work like that, just long lived threads keep stale references to objects that should have been long gone. If the thread dies everything is well and fine but threadpools/designated threads can live until the end of the process while many others objects/classloaders can by dynamically loaded/unloaded.

[bestsss]

@Jim, not removing the object via either set(null) or remove() and the thread keeps going would be the thing causing the leak. There is absolutely no need to call set prior to set(xxx).

[bestsss]

The entry in the ThreadLocalMap extends WeakReference<ThreadLocal> and the value has a HARD ref in the ThreadLocalMap.Entry[] -- since the value will have a reference to the classloader and the classloader to the ThreadLocal --> #1 leak offender. You need dynamic classloading to exploit it fully, though.

[bestsss]

@irreputable, yeah I never extend ThreadLocals and keep non-bootstap/system classloder loaded classes in there unless in try/finally{remove}. Link to my guidelines to avoid leaks.

[irreputable]

@bestsss recent jdk's ThreadLocalMap references keys weakly, so it should be fine. are you worried about other ThreadLocal impls?

[Tomasz]

This answer is now outdated. The current implementation of ThreadLocal has a per-thread map of values. From JavaDoc: "after a thread goes away, all of its copies of thread-local instances are subject to garbage collection".