Jianhua Shi
Jianhua Shi

Reputation: 21

Threadlocal Memory Leak

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?

Upvotes: 2

Views: 2766

Answers (2)

Manyata Goyal
Manyata Goyal

Reputation: 614

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.

Upvotes: 1

walen
walen

Reputation: 7273

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.

Upvotes: 6

Related Questions