A DESCRIPTION OF THE REQUEST :
Provide a guaranteed method of creating thread local variables from within ThreadLocal.initialValue().

JUSTIFICATION :
Currently thread local variables created from within ThreadLocal.initialValue() are not guaranteed to live after the ThreadLocal.get() method completes. That is because get() creates a new ThreadLocalMap possibly overwriting any thread local variables that may have been created from within initialValue().

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
    public Object get()
    {
        Thread t = Thread.currentThread();
        
        ThreadLocalMap map = getMap(t);

        if (map != null)
            return map.get(this);

        Object value = initialValue();

        ThreadLocalMap map = getMap(t);

        if (map != null)
            map.set(this, value);
        else
            createMap(t, value);

        return value;
    }

ACTUAL -
    public Object get()
    {
        Thread t = Thread.currentThread();
        
        ThreadLocalMap map = getMap(t);

        if (map != null)
            return map.get(this);

        Object value = initialValue();

        createMap(t, value);

        return value;
    }


---------- BEGIN SOURCE ----------
JUnit / relection uses ThreadLocal I believe, so I had to resort to a POJO test.

package junit;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

/**
 * @author Harish Krishnaswamy
 * @version $Id: $
 * @since 1.0
 */
public class TestThreadLocal
{
    private static class TestThreadEventNotifier
    {
        private ThreadLocal _local = new ThreadLocal();

        private List getListeners()
        {
            List list = (List) _local.get();

            if (list == null)
            {
                list = new ArrayList();
                _local.set(list);
            }

            return list;
        }

        public void addListener(TestThreadLocalStorage listener)
        {
            List list = getListeners();

            list.add(listener);
        }

        public void fireThreadCleanup()
        {
            List list = getListeners();

            for (Iterator itr = list.iterator(); itr.hasNext();)
            {
                TestThreadLocalStorage listener = (TestThreadLocalStorage) itr.next();

                listener.cleanup();
            }
        }
    }

    private static class TestThreadLocalStorage
    {
        private CleanableThreadLocal _local;

        TestThreadLocalStorage(TestThreadEventNotifier notifier)
        {
            _local = new CleanableThreadLocal(this, notifier);
        }

        private class CleanableThreadLocal extends ThreadLocal
        {
            private TestThreadLocalStorage  _storage;
            private TestThreadEventNotifier _notifier;

            private CleanableThreadLocal(TestThreadLocalStorage storage,
                TestThreadEventNotifier notifier)
            {
                _storage = storage;
                _notifier = notifier;
            }

            protected Object initialValue()
            {
                if (_notifier != null && _storage != null)
                    _notifier.addListener(_storage);

                return new HashMap();
            }
        }

        public Object get(Object key)
        {
            Map map = (Map) _local.get();

            return map.get(key);
        }

        public void put(Object key, Object value)
        {
            Map map = (Map) _local.get();

            map.put(key, value);
        }

        public void cleanup()
        {
            Map map = (Map) _local.get();

            map.clear();
        }

    }

    public static void main(String[] args)
    {
        TestThreadEventNotifier notifier = new TestThreadEventNotifier();
        TestThreadLocalStorage storage = new TestThreadLocalStorage(notifier);

        storage.put("mykey", "myvalue");

        notifier.fireThreadCleanup();

        Object myVal = storage.get("mykey");
        
        if (myVal == null)
            System.out.println("ThreadLocal values successfully cleared!");
        else
            System.out.println("ERROR: ThreadLocal values not cleared: " + myVal);
    }
}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
package junit;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

/**
 * @author Harish Krishnaswamy
 * @version $Id: $
 * @since 1.0
 */
public class TestThreadLocalWorkaround
{
    private static class TestThreadEventNotifier
    {
        private ThreadLocal _local = new ThreadLocal();

        private List getListeners()
        {
            List list = (List) _local.get();

            if (list == null)
            {
                list = new ArrayList();
                _local.set(list);
            }

            return list;
        }

        public void addListener(ThreadLocalStorage listener)
        {
            List list = getListeners();

            list.add(listener);
        }

        public void fireThreadCleanup()
        {
            List list = getListeners();

            for (Iterator itr = list.iterator(); itr.hasNext();)
            {
                ThreadLocalStorage listener = (ThreadLocalStorage) itr.next();

                listener.cleanup();
            }
        }
    }

    private static class ThreadLocalStorage
    {
        private static final String     INITIALIZED_KEY = "$init$";

        private CleanableThreadLocal    _local          = new CleanableThreadLocal();
        private TestThreadEventNotifier _notifier;

        ThreadLocalStorage(TestThreadEventNotifier notifier)
        {
            _notifier = notifier;
        }

        private class CleanableThreadLocal extends ThreadLocal
        {
            protected Object initialValue()
            {
                Map map = new HashMap();
                map.put(INITIALIZED_KEY, Boolean.TRUE);

                return map;
            }
        }

        private Map getThreadLocalVariable()
        {
            Map map = (Map) _local.get();

            if (Boolean.TRUE.equals(map.get(INITIALIZED_KEY)) && _notifier != null)
            {
                _notifier.addListener(this);

                map.remove(INITIALIZED_KEY);
            }

            return map;
        }

        public Object get(Object key)
        {
            Map map = getThreadLocalVariable();

            return map.get(key);
        }

        public void put(Object key, Object value)
        {
            Map map = getThreadLocalVariable();

            map.put(key, value);
        }

        public void cleanup()
        {
            Map map = (Map) _local.get();

            map.clear();
        }

    }

    public static void main(String[] args)
    {
        TestThreadEventNotifier notifier = new TestThreadEventNotifier();
        ThreadLocalStorage storage = new ThreadLocalStorage(notifier);

        storage.put("mykey", "myvalue");

        notifier.fireThreadCleanup();

        Object myVal = storage.get("mykey");
        
        if (myVal == null)
            System.out.println("ThreadLocal values successfully cleared!");
        else
            System.out.println("ERROR: ThreadLocal values not cleared: " + myVal);
    }
}
(Incident Review ID: 241038) 
======================================================================
Posted Date : 2005-07-27 04:38:59.0

N/A

This RFE will deliver useability of the API in certain recursive settings in return for a relatively simple change that is performance neutral and results in slightly simpler code.
  xxxxx@xxxxx   2005-04-26 19:01:20 GMT

This is actually a bug, not an RFE.

Yup, it

Yup, it

Yup, it's a bug, but difficult to see from the way it is presented.

The example code appears to work if started via the command line java command. However, the proposed change does make sense.

If an implementation of initialValue causes another ThreadLocal instance's initialValue to be caused, then we have two ThreadLocalMaps. The map created by the first ThreadLocal overwrite that made by the second, without rechecking after running 'user code'.

Here's a quick test. It prints the same ThreadLocal's value twice. That should be "1 1", but because it is initialised twice "1 2" is printed. I've removed the generics so it runs on older versions. 1.3 passes this test; 1.4 and 1.5 are not as successful.

class ThreadLocalInitBug {
    static class Outside extends ThreadLocal {
        protected Object initialValue() {
            ThreadLocal inside = new Inside();
            System.out.print(inside.get()+" ");
            return inside;
        }
    }
    static class Inside extends ThreadLocal {
        private int counter;
        protected Object initialValue() {
            return new Integer(++counter);
        }
    }
    public static void main(String[] args) {
        // Loading classes makes use of thread locals
        //   (a SoftReference to a StringCoding)
        //   and therefore initialises the thread's locals map.
                                                                                
        // Makes sure classes are loaded.
        //   (We could just do it twice.)
        new Outside(); new Inside();
        
        // main thread may (will) have already initialised some ThreadLocal.
        new Thread(new Runnable() { public void run() {
                    ThreadLocal outside = new Outside();
                    System.out.println(((ThreadLocal)outside.get()).get());
                                                                                
        }}).start();
   }
}

A simple way of fixing the issue is to replace 

        createMap(t, value);

in ThreadLocal.get with

        set(value);
        
For performance reasons, as this is the unusual case (if it isn't and needs to optimisation, then you have other problems to worry about), the additional code to ThreadLocal.get should be minimised.

A pitfall for Josh's new book. ;)

The call to initialValue for subsequent ThreadLocals in a Thread within ThreadLocalMap.getAfterMiss is also broken. The map look-up should be started afresh after invoking initialValue.

As a test case:

import java.util.*;
 
class ThreadLocalDrop {
    private static class Local extends ThreadLocal {
        private int depth;
        private List locals;
        private int counter;
        public Local(int depth, List locals) {
            this.depth = depth;
            this.locals = locals;
            locals.add(this);
        }
        @Override
        public Object initialValue() {
            System.err.print('.');
            if (counter == 1) {
                //throw new Error("ThreadLocal initialised twice");
            }
            if (depth-1 > 0) {
                new Local(depth-1, locals).get();
            }
            return new Integer(++counter);
        }
    }
    public static void main(String[] args) {
        final int depth = args.length<=0 ? 3 : Integer.parseInt(args[0]);
        final int runs = 10000/depth;
        List dump = new ArrayList();
        for (int outerCt=0; outerCt<runs; ++outerCt) {
            List locals = new ArrayList();
            new Local(depth, locals).get();
            dump.addAll(locals);
            for (int ct=0; ct<depth; ++ct) {
                System.err.print("-");
                ((ThreadLocal)locals.get(ct)).get();
            }
            System.err.println();
        }
    }
}

Should print 3 columns of dots and 3 of dashes. Instead, some of the lines have extra dots, representing incorrect second calls to initialValue.

...---
...---
...-...--
...---
...---
...-...-..-
...---
...---
...---
...