JDK-6558265 : (thread) Using thread locals with thread pools may lead to unintentional object retention
  • Type: Enhancement
  • Component: core-libs
  • Sub-Component: java.lang
  • Affected Version: 7
  • Priority: P3
  • Status: Open
  • Resolution: Unresolved
  • OS: windows_xp
  • CPU: x86
  • Submitted: 2007-05-16
  • Updated: 2015-06-01
Related Reports
Duplicate :  
Relates :  
Relates :  
Relates :  
Description
A DESCRIPTION OF THE REQUEST :
In case when one worker thread is reused, it may happen that this thread would accumulate thread locals from executed tasks, which could not be used anymore

or, even worse, prevent GC collect objects that they hold. There is should be a way to tear off accumulated thread locals (as a whole) on task completion.

Also It would be good if task would work in clean context and use inheritableThreadLocals from thread that created and scheduled the task, but not from

thread that created workers; i.e. as if it works in newly created thread.

This example demonstrates problem (OOME):
public class Main {
	private static final int HUGE_BLOCK_SIZE = (int) (Runtime.getRuntime().maxMemory() / 2 + 1);

	private static ThreadLocal tls = new ThreadLocal<Object>() {
		protected Object initialValue() {
			System.out.println(Thread.currentThread());
			return new byte[HUGE_BLOCK_SIZE];
		}
	};

	private static ExecutorService executor = Executors.newFixedThreadPool(2);
	static void execute(Runnable task) throws ExecutionException, InterruptedException {
		System.out.println("begin");
		Runnable r = task;
		Future<?> future = executor.submit(r);
		future.get(); // wait for completion
		System.out.println("end");
	}

	public static void main(String[] args) throws Exception {
		Runnable task = new Runnable() {
			public void run() {
				tls.get();
			}
		};
		try {
			// the first time the task is executed in worker #1
			// huge object is created and put it to threadLocals of worker #1
			execute(task);
			// the second time the task is executed in worker #2
			// huge object cannot be created since worker #1 still holds its threadLocals
			execute(task);
		} finally {
			executor.shutdown();
		}
	}

}

Please note that no problem appears if method execute() would be implemented in way that always creates new worker thread. I.e. like this:
	static void execute(Runnable task) throws InterruptedException {
		System.out.println("begin");
		Thread thread = new Thread(task);
		thread.start();
		thread.join();
		System.out.println("end");
	}

I propose to create class ThreadLocalScope, which has to clean up thread locals that are created during task execution. It should be possible to use it in

following way:
	static void execute(Runnable task) throws ExecutionException, InterruptedException {
		System.out.println("begin");
		Runnable r = new ThreadLocalScope(task);
		Future<?> future = executor.submit(r);
		future.get(); // wait for completion
		System.out.println("end");
	}

Here is a sample of ThreadLocalScope implementation:
package java.lang;

/**
 * <p>The <tt>ThreadLocalScope</tt> is to prevent polution of map of thread locals,
 * which could take place when one or more worker threads are reused to
 * execute several tasks. In general, after task completion it restores that thread locals
 * that were before the task were executed.</p>
 *
 * <p>The <tt>ThreadLocalScope</tt> could be used in two cases:
 * <ol>
 * <li>As a decorator of task that are to be executed in separate thread
 * <pre>
 * Runnable task = ...
 * threadPool.execute(new ThreadLocalScope(task));
 * </pre>
 * </li>
 * <li>As an executor of task in current thread
 * <pre>
 * Runnable task = ...
 * ThreadLocalScope.run(task);
 * </pre>
 * </li>
 * </ol>
 * </p>
 *
 * <p>Also the <tt>ThreadLocalScope</tt> can work in two modes:
 * <ol>
 * <li>task is executed as it is in a newly created thread. I.e. inheritable thread locals
 * are inherited from the thread that created instance of <tt>ThreadLocalScope</tt> and non-inheritable thread locals are empty.</li>
 * <li>task is executed as it is in a worker thread but changes
 * in thread locals are to be reverted anyway</li>
 * </ol>
 * </p>
 */
public class ThreadLocalScope implements Runnable {
	private final Runnable _target;
	private final boolean _mode;
	private final ThreadLocal.ThreadLocalMap _heritage;

	/**
	 * Similar to
	 * <pre>
	 * new ThreadLocalScope(true, target);
	 * </pre>
	 * @see ThreadLocalScope#ThreadLocalScope(boolean, Runnable)
	 * @param target decorated task
	 * @throws NullPointerException if target is null
	 */
	public ThreadLocalScope(Runnable target) {
		this(true, target);
	}

	/**
	 * @param inheritThreadLocalsFromCreator mode, if it is true the task is executed as it is in a newly created thread,
	 * otherwise the task is executed as it is in a worker thread.
	 * @param target decorated task
	 * @throws NullPointerException if target is null
	 */
	public ThreadLocalScope(boolean inheritThreadLocalsFromCreator, Runnable target) {
		if (target == null)
			throw new NullPointerException("target");
		_target = target;
		_mode = inheritThreadLocalsFromCreator;
		_heritage = inheritThreadLocalsFromCreator? copy(Thread.currentThread().inheritableThreadLocals): null;
	}

	static ThreadLocal.ThreadLocalMap copy(ThreadLocal.ThreadLocalMap map) {
		return (map == null)? null: ThreadLocal.createInheritedMap(map);
	}

	public void run() {
		run(_heritage, _mode, _target);
	}

	/**
	 * Executes task in current thread.
	 * Similar to
	 * <pre>
	 * ThreadLocalScope.run(false, target);
	 * </pre>
	 * @param target decorated task
	 * @throws NullPointerException if target is null
	 */
	public static void run(Runnable target) {
		run(false, target);
	}

	/**
	 * Executes task in current thread.
	 * @param inheritThreadLocalsFromCreator mode, if it is null the task is executed as it is in a newly created thread
	 * (i.e. non-inheritable thread locals are empty), the task is executed as it is in current thread (i.e. all thread locals
	 * are accessible but any changes are reverted after task completion).
	 * @param target decorated task
	 * @throws NullPointerException if target is null
	 */
	public static void run(boolean inheritThreadLocalsFromCreator, Runnable target) {
		if (target == null)
			throw new NullPointerException("target");
		ThreadLocal.ThreadLocalMap heritage = inheritThreadLocalsFromCreator? Thread.currentThread().inheritableThreadLocals: null;
		run(heritage, inheritThreadLocalsFromCreator, target);
	}

	static void run(ThreadLocal.ThreadLocalMap heritage, boolean inheritThreadLocalsFromCreator, Runnable target) {
		Thread current = Thread.currentThread();
		ThreadLocal.ThreadLocalMap inheritableThreadLocals = current.inheritableThreadLocals;
		ThreadLocal.ThreadLocalMap threadLocals = current.threadLocals;
		if (inheritThreadLocalsFromCreator) {
			current.inheritableThreadLocals = copy(heritage);
			current.threadLocals = null;
		} else {
			ThreadLocal.ThreadLocalMap tmp = copy(inheritableThreadLocals);
			current.threadLocals = copy(threadLocals);
			current.inheritableThreadLocals = tmp;
		}
		try {
			target.run();
		} finally {
			current.inheritableThreadLocals = inheritableThreadLocals;
			current.threadLocals = threadLocals;
		}
	}

}


JUSTIFICATION :
see description

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
begin
Thread[Thread-0,5,main]
end
begin
Thread[Thread-1,5,main]
end

ACTUAL -
begin
Thread[pool-1-thread-1,5,main]
end
begin
Thread[pool-1-thread-2,5,main]
Exception in thread "main" java.util.concurrent.ExecutionException: java.lang.OutOfMemoryError: Java heap space
	at java.util.concurrent.FutureTask$Sync.innerGet(Unknown Source)
	at java.util.concurrent.FutureTask.get(Unknown Source)
	at org.shejko.tlscope.Main.execute(Main.java:42)
	at org.shejko.tlscope.Main.main(Main.java:58)
Caused by: java.lang.OutOfMemoryError: Java heap space


---------- BEGIN SOURCE ----------
see description
---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
1. avoid using thread pools for tasks that use thread locals
2. use explicit ThreadLocal.remove() method when task is completed, this is not always practical since some thread locals could be created in internals of third-party libraries and it is not common practice to expose instances of ThreadLocal for clean up.

Comments
EVALUATION The provided ThreadLocalScope is very interesting. The biggest issue I can see with this is "invasion of privacy". ThreadLocals in "foreign" code cannot be accessed without resorting to reflection. But ThreadLocalScope creates an environment where other code's thread local variables have been tampered with (true, only temporarily, and only in the specified Runnable). This might be unexpected. ThreadLocals are often used to transmit information through a foreign API that calls back into "your" code. Sleazy, but occasionally necessary. There might also be security implications to being able to change ThreadLocal values without holding a reference to same. I can imagine another solution. Suppose ThreadLocalScope instructed the ThreadLocalMap implementations to keep track of all modifications to ThreadLocals on a stack. After the task completed, the stack could be unwound by popping modifications and undoing them. This would also solve the memory leak problem without making such a drastic change in the map contents. But it would need cooperation from the implementations of ThreadLocal. Another possibility is to simply copy and restore the ThreadLocalMaps around the run(), although doing so unconditionally might be more expensive than one would like. Another possibility is to implement the above, but only make it available for use with ThreadPoolExecutor, eliminating the possibility of abuse.
15-11-2007

EVALUATION Contribution forum : https://jdk-collaboration.dev.java.net/servlets/ProjectForumMessageView?forumID=1463&messageID=20016
21-05-2007

WORK AROUND Use ThreadLocal.remove (for the "main bug"). Avoiding inheritance of InheritableThreadLocals is impossible: there is no workaround except to avoid thread pools.
16-05-2007