Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is it dangerous to use ThreadLocal with ExecutorService?

I was going through the concept of ThreadLocals on the below blog :

https://www.baeldung.com/java-threadlocal

It says that "Do not use ThreadLocal with ExecutorService"

It illustrates below example for using ThreadLocals.

public class ThreadLocalWithUserContext implements Runnable {
  
    private static ThreadLocal<Context> userContext 
      = new ThreadLocal<>();
    private Integer userId;
    private UserRepository userRepository = new UserRepository();
 
    @Override
    public void run() {
        String userName = userRepository.getUserNameForUserId(userId);
        userContext.set(new Context(userName));
        System.out.println("thread context for given userId: "
          + userId + " is: " + userContext.get());
    }
     
    // standard constructor
}

At the End of the post it mentions that :

If we want to use an ExecutorService and submit a Runnable to it, using ThreadLocal will yield non-deterministic results – because we do not have a guarantee that every Runnable action for a given userId will be handled by the same thread every time it is executed.

Because of that, our ThreadLocal will be shared among different userIds. That’s why we should not use a TheadLocal together with ExecutorService. It should only be used when we have full control over which thread will pick which runnable action to execute.

This explanation was a bouncer to me. I tried to do some research online for this point specifically but I could not get much help, can some expert please elaborate on the above explanation? Is it authors view or a real Threat?

like image 796
Hitesh Avatar asked Feb 16 '19 17:02

Hitesh


1 Answers

ThreadLocal is used when you want to cache a variable in that thread after you set it. So next time when you want to access it, you can just directly get it from the ThreadLocal without initialization.

Since you set it with threadLocal.set(obj) and you access it via threadLocal.get() within a thread, so directly you have thread-safe guarantee.

But things might become ugly, if you do not clear the cache by threadLocal.remove() explicitly.

  1. in a thread pool, the tasks queued up will be processed by threads one by one and the task should be independent most of the time, but the thread-scope cache threadLocal will make the following tasks depend on its previous if you forget to clear it first before handling the next task;

  2. the cached threadLocals will not be gc-ed immediately (at some unknown moment - out of your control) since their keys are WeakReference, which might cause OOM without your knowing.

A simple demo for such case where remove() is not explicitly invoked causing an OOM.

public class ThreadLocalOne {
    private static final int THREAD_POOL_SIZE = 500;
    private static final int LIST_SIZE = 1024 * 25;

    private static ThreadLocal<List<Integer>> threadLocal = new ThreadLocal<>();

    public static void main(String[] args) throws InterruptedException {

        ExecutorService executorService = Executors.newFixedThreadPool(THREAD_POOL_SIZE);

        for (int i = 0; i < THREAD_POOL_SIZE; i++) {
            executorService.execute(() -> {
                threadLocal.set(getBigList());
                System.out.println(Thread.currentThread().getName() + " : " + threadLocal.get().size());
                // threadLocal.remove(); 
                // explicitly remove the cache, OOM shall not occur;
            });
        }
        executorService.shutdown();
    }

    private static List<Integer> getBigList() {
        List<Integer> ret = new ArrayList<>();
        for (int i = 0; i < LIST_SIZE; i++) {
            ret.add(i);
        }
        return ret;
    }
}
like image 119
Hearen Avatar answered Oct 16 '22 05:10

Hearen