1

I see someone said: "when you want to use ThreadLocal in your class please use it in static way",such as :

private static ThreadLocal<SimpleDateFormat> dayFormat = 
    new ThreadLocal<SimpleDateFormat>() { 
        protected SimpleDateFormat initialValue() {
            return new SimpleDateFormat("yyyy-MM-dd"); 
        }
    };

I am not sure why this can avoid memory leak. Can anybody give some clarifications?

Samurai
  • 3,724
  • 5
  • 27
  • 39
  • 4
    What "memory leak"? Anyway, it's [most] always static simply because it generally doesn't make sense to be per-instance. – user2864740 Jun 23 '15 at 01:37
  • 2
    Can you not ask the person who told you that? – Dour High Arch Jun 23 '15 at 01:42
  • @DourHighArch, the architect told me don't use it in none-static way,otherwise it will caused memory leak. – Jianhua Shi Jun 23 '15 at 01:47
  • @user2864740,I see some post said the key is weak reference which can be GC,however the value is a strong reference which can't be GC. See details in : http://javarevisited.blogspot.com.au/2013/01/threadlocal-memory-leak-in-java-web.html – Jianhua Shi Jun 23 '15 at 01:52
  • 2
    Your architect probably misspoke, there wouldn't be a memory leak. As user2864740 says, it's just that normally, when you use ThreadLocal, you want to have a single value per thread. But if you don't mark it as static, then you have one value per instance-per thread. Not only is it probably wrong, but you would in fact be consuming more memory. But it's not a leak. – sstan Jun 23 '15 at 02:01
  • Thanks for clarification.@Samurai. – Jianhua Shi Jun 23 '15 at 02:04

2 Answers2

5

I coded some PoCs and launched jvisualvm to actually show whether a static ThreadLocal is any different from an instance ThreadLocal.
The code starts 10 different threads, each running a hundred-million-iteration loop that makes use of the SimpleDateFormat object stored in the ThreadLocal field.

Static ThreadLocal

public class Main {
    public static void main(String[] args) {
        for (int i = 0; i < 10; i++) {
            new Thread(() -> {
                System.out.println(Thread.currentThread().getName() + " " + dayFormat.get().format(new Date()));
                for (int j = 0; j < 100000000; j++) {
                    dayFormat.get().format(new Date());
                }
                System.out.println(Thread.currentThread().getName()+" "+dayFormat.get().format(new Date()));
            }).start();
        }
    }

    private static ThreadLocal<SimpleDateFormat> dayFormat =
            new ThreadLocal<SimpleDateFormat>() {
                protected SimpleDateFormat initialValue() {
                    return new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
                }
            };
}

static not null

No memory leaks here, as expected.

Instance ThreadLocal

public class Main {
    public static void main(String[] args) {
        for (int i = 0; i < 10; i++) {
            new Thread(() -> {
                Main m = new Main();
                System.out.println(Thread.currentThread().getName() + " " + m.dayFormat.get().format(new Date()));
                for (int j = 0; j < 100000000; j++) {
                    m.dayFormat.get().format(new Date());
                }
                System.out.println(Thread.currentThread().getName()+" "+m.dayFormat.get().format(new Date()));
            }).start();
        }
    }

    private ThreadLocal<SimpleDateFormat> dayFormat =
            new ThreadLocal<SimpleDateFormat>() {
                protected SimpleDateFormat initialValue() {
                    return new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
                }
            };
}

instance, not null

No memory leaks here, either. It does run significantly (about 20%) faster than the static version, but there's not much difference in the memory footprint.

However, since that code did not ever "nullify" the ThreadLocal's reference to the object, we don't know how the GC feels about all this.
The following code does.

Static ThreadLocal, modified during run time

public class Main {
    public static void main(String[] args) {
        for (int i = 0; i < 10; i++) {
            new Thread(() -> {
                System.out.println(Thread.currentThread().getName() + " " + dayFormat.get().format(new Date()));
                for (int j = 0; j < 100000000; j++) {
                    dayFormat.set(null);
                    dayFormat.set(new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"));
                    dayFormat.get().format(new Date());
                }
                System.out.println(Thread.currentThread().getName()+" "+dayFormat.get().format(new Date()));
            }).start();
        }
    }

    private static ThreadLocal<SimpleDateFormat> dayFormat =
            new ThreadLocal<SimpleDateFormat>() {
                protected SimpleDateFormat initialValue() {
                    return new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
                }
            };
}

static, nullified

Instance ThreadLocal, modified during run time

public class Main {
    public static void main(String[] args) {
        for (int i = 0; i < 10; i++) {
            new Thread(() -> {
                Main m = new Main();
                System.out.println(Thread.currentThread().getName() + " " + m.dayFormat.get().format(new Date()));
                for (int j = 0; j < 100000000; j++) {
                    m.dayFormat.set(null);
                    m.dayFormat.set(new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"));
                    m.dayFormat.get().format(new Date());
                }
                System.out.println(Thread.currentThread().getName()+" "+m.dayFormat.get().format(new Date()));
            }).start();
        }
    }

    private ThreadLocal<SimpleDateFormat> dayFormat =
            new ThreadLocal<SimpleDateFormat>() {
                protected SimpleDateFormat initialValue() {
                    return new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
                }
            };
}

instance, nullified

The memory footprint is once again very similar in both cases. This time there's some kind of pause in the GC, which leads to a "bulge" in the memory graph, but it appears both in the static version and in the instance version.
Run times are somewhat similar, with the instance version being ever so slightly (about 5%) slower this time.

So, as expected by most people here, there doesn't seem to be any risk of memory leaks if you declare your ThreadLocal objects as instance fields instead of static fields.

walen
  • 7,103
  • 2
  • 37
  • 58
1

I read the same thing on another stack overflow answer. As it explains, in a "general" usecase you want a threadlocal to be per thread and hence it works as a thumb rule.

I have however seen code which necessitates multiple instances of the threadlocal variable in the same thread in which case it is declared non static.

The possible memory leak in this case comes from the fact, that the impact of forgetting to remove each instance of the thread local when the job is done (if you are using thread pools where threads dont die and hence threadlocals dont get garbage collected), this can cause a bloat in the memory usage causing a memory leak.

Manyata Goyal
  • 604
  • 5
  • 13