20

My limited understanding of ThreadLocal is that it has resource leak issues. I gather this problem can be remedied through proper use of WeakReferences with ThreadLocal (although I may have misunderstood this point.) I would simply like a pattern or example for correctly using ThreadLocal with WeakReference, if one exists. For instance, in this code snippet where would the WeakReference be introduced?

static class DateTimeFormatter {
    private static final ThreadLocal<SimpleDateFormat> DATE_PARSER_THREAD_LOCAL = new ThreadLocal<SimpleDateFormat>() {
        protected SimpleDateFormat initialValue() {
            return new SimpleDateFormat("yyyy/MM/dd HH:mmz");
        }
    };
    public String format(final Date date) {
        return DATE_PARSER_THREAD_LOCAL.get().format(date);
    }
    public Date parse(final String date) throws ParseException
    {
      return DATE_PARSER_THREAD_LOCAL.get().parse(date);
    }
}
Julien Chastang
  • 17,592
  • 12
  • 63
  • 89

8 Answers8

37

ThreadLocal uses a WeakReference internally. If the ThreadLocal is not strongly referenced, it will be garbage-collected, even though various threads have values stored via that ThreadLocal.

Additionally, ThreadLocal values are actually stored in the Thread; if a thread dies, all of the values associated with that thread through a ThreadLocal are collected.

If you have a ThreadLocal as a final class member, that's a strong reference, and it cannot be collected until the class is unloaded. But this is how any class member works, and isn't considered a memory leak.


Update: The cited problem only comes into play when the value stored in a ThreadLocal strongly references that ThreadLocal—sort of a circular reference.

In this case, the value (a SimpleDateFormat), has no backwards reference to the ThreadLocal. There's no memory leak in this code.

erickson
  • 265,237
  • 58
  • 395
  • 493
  • Leaking when a thread-local value (indirectly) references its ThreadLocal is still a bug in Sun's implementaiton. I'm not sure if crazy Bob Lee's implementation in Harmony has the same problem. – Tom Hawtin - tackline Jun 02 '09 at 17:29
  • Is it a bug in ThreadLocal, or the application and its use of ThreadLocal? I haven't run into it yet (and I try to avoid ThreadLocal), so I'm still not sure. – erickson Jun 02 '09 at 17:51
  • 1
    Note that in a Servlet environment you NEED to use a servlet filter in order to clear that ThreadLocal, if you don't do that you will have a lot of leaks on redeploys – Nestor Hernandez Loli Jul 13 '14 at 06:30
11

I'm guessing you're jumping through these hoops since SimpleDateFormat is not thread-safe.

Whilst I'm aware I'm not solving your problem above, can I suggest you look at Joda for your date/time work ? Joda has a thread-safe date/time formatting mechanism. You won't be wasting your time learning the Joda API either, as it's the foundation for the new standard date/time API proposal.

Brian Agnew
  • 268,207
  • 37
  • 334
  • 440
  • 1
    +1: I didn't even think of that. It's a fair bet that this is the case, if not for this OP, then possibly for someone else. – Alex Beardsley Jun 02 '09 at 16:49
4

There shouldn't be such a problem.

A thread's ThreadLocal reference is defined to exist only as long as the corresponding thread is alive (see the javadoc)-- or put another way, once the thread is not alive, if the ThreadLocal was the only reference to that object, then the object becomes eligible for garbage collection.

So either you've found a genuine bug and should be reporting it, or you're doing something else wrong!

Neil Coffey
  • 21,615
  • 7
  • 62
  • 83
  • 2
    Consider the case when you're running inside an app server/container which can unload your application (to install, say, a different version) but continue to use the threads for a new app release. – Charles Duffy Apr 09 '12 at 21:38
3

I realise this isn't strictly an answer to your question but as a general rule I won't suggest using a ThreadLocal in situration where there isn't a clear tear down at the end of a request/interaction. The classic is doing this sort of thing in a servlet container, at first glance it seems fine, but since the threads are pooled it becomes a issue with the ThreadLocal hanging onto the resource even after each request has been processed.

Suggestions:

  1. Use a filter of similar wrapper for each interaction to clear the ThreadLocal at the end of each interaction
  2. You could use a alternative to SimpleDateFormat like FastDateFormat from commons-lang or Joda as somebody has already suggested
  3. Just create a new SimpleDateFormat every time you need it. Seems wasteful I know, but in most applications you just won't notice the difference
reevesy
  • 3,452
  • 1
  • 26
  • 23
Gareth Davis
  • 27,701
  • 12
  • 73
  • 106
1

Just to add on to what @Neil Coffey said, this shouldn't be a problem as long as your ThreadLocal instance is static. Because you keep calling get() on a static instance, it should always hold the same reference to your simple date formatter. Therefore, as Neil said, when the Thread is terminated, the only instance of simple date formatter should be eligible for garbage collection.

If you have numbers or some other form of debugging that shows this introducing resource issues, then that's another story. But I believe as is it shouldn't be a problem.

Alex Beardsley
  • 20,988
  • 15
  • 52
  • 67
0

clean the thread local after using it, add servlet filter to do so:

protected ThreadLocal myThreadLocal = new ThreadLocal();

public void doFilter (ServletRequest req, ServletResponse res, chain) throws IOException, ServletException {
    try {
        // Set ThreadLocal (eg. to store heavy computation result done only once per request)
        myThreadLocal.set(context());

        chain.doFilter(req, res);
    } finally {
        // Important : cleanup ThreaLocal to prevent memory leak
        userIsSmartTL.remove();
    }
}
legoscia
  • 39,593
  • 22
  • 116
  • 167
Nadav Finish
  • 1,120
  • 9
  • 7
0

The above code example has no problem since it uses the ThreadLocal in a static variable.

One problem with ThreadLocal memory leaks occurs when you initialize instances of ThreadLocals as non static variables. When the object holding that variable is garbage collected, the reference of the ThreadLocal stays in the thread. If you then instantiate and use many ThreadLocals in some kind of loop, you get a memory leak.

I had this problem with FastThreadLocal from netty (and I guess java ThreadLocal should have the same problem). My solution is to use weak referenced map values inside a ThreadLocal to work around this issue. This allows to use ThreadLocal variables as instance variables that can be garbage collected when the holding object is freed.

Here the code (can be used in place of ThreadLocals): https://github.com/invesdwin/invesdwin-util/blob/master/invesdwin-util-parent/invesdwin-util/src/main/java/de/invesdwin/util/concurrent/reference/WeakThreadLocalReference.java

subes
  • 1,832
  • 5
  • 22
  • 28
0

In your example there should be no problem using a ThreadLocal at all.

Thread locals (and singletons also!) become a problem when thread locals are set to instances loaded by a classloader to be unloaded later. A typical situation in a servlet container (like tomcat):

  1. A webapp set a thread local during request processing.
  2. The thread is managed by the container.
  3. The webapp should be undeployed.
  4. The classloader of the webapp cannot be garbaged, because there is a reference left: from the thread local to the instance to its class to its classloader.

The same with singletons (java.sql.DriverManger and JDBC drivers offered by the webapp).

So avoid such things in particular in environments not under your full control!

Arne Burmeister
  • 20,046
  • 8
  • 53
  • 94
  • 3
    No, Formatters are not thread safe. Multiple threads invoking the parse method of a single instance is a disaster. – erickson Jun 02 '09 at 16:59
  • ok, didnt thought about SimpleDateFormat, but there should be no problem with the ThreadLocal anyway. – Arne Burmeister Jun 03 '09 at 06:14
  • See also [Tomcat's Memory Leak Protection](http://wiki.apache.org/tomcat/MemoryLeakProtection). They also describe this ThreadLocal leak, but unfortunately do not suggest a workaround. It seems Tomcat itself attempts to nullify all static class references in a webapp's classloader classes when that webapp is undeployed. – Patrick Mar 03 '12 at 13:58