Using ThreadLocal with existing classes which contain static members

Question

I'm trying to use ThreadLocal to provide thread safety to pre-existing non-thread-safe classes, but experiencing problems. It appears that there is no isolation being performed - that the threads still share the static, instead of it being local to each thread.

I believe my usage is almost exactly parallel with the example localization of SimpleDateFormatter described in this StackOverflow question, but it's not working out the way I'd hope.

What I'm hoping for is that someone out there who's used this will point out the egregiously clueless error I must be making... so I guess my question is: can you spot what I'm doing wrong here?

Here's my simple class:

public class SimpleClassWithStaticMembers {
    private static String theStaticString =
        "StaticStringInClassWithStaticMember";
    public void setTheStaticString (String val) {
        SimpleClassWithStaticMembers.theStaticString = val; 
    }
    public String getTheStaticString () {
        return SimpleClassWithStaticMembers.theStaticString;
    }
}

And this is the thread class which creates threadlocal instances of SimpleClassWithStaticMembers:

public class SimpleTesterThread extends Thread {
    private void showMsg (String msg) {  
        System.out.println (msg); 
        System.out.flush(); 
    }
    public SimpleTesterThread (String threadId) {
        super(threadId);
    }
    public void run() {
        try { Thread.sleep(2000); } catch (InterruptedException ex) { }
        ThreadLocal<SimpleClassWithStaticMembers> localizedClass =
            new ThreadLocal<SimpleClassWithStaticMembers>();
        localizedClass.set(new SimpleClassWithStaticMembers());
        // repeating here to be sure we overlap all with all
        for (int ii=0; ii < 3; ii++) { 
            localizedClass.get().setTheStaticString ("Setby_" + this.getName());
            try { Thread.sleep(2000); } catch (InterruptedException ex) { }
            showMsg("            Thread [" + this.getName() + "] - " 
                + localizedClass.get().getTheStaticString() + "'.");
        }
        showMsg ("Thread [" + this.getName() 
            + "] complete. Our 'threadlocal' string is now - " 
            + localizedClass.get().getTheStaticString() + "'.");
        localizedClass.remove();
    }
}

When ten instances of SimpleTesterThread are created (giving them distinct thread names like "AAAAAAAAAA", "BBBBBBBBB", etc), and then started, the output clearly shows that they're sharing instances. Log output includes:

...
            Thread [JJJJJJJJJJ] - Setby_CCCCCCCCCC'.
            Thread [DDDDDDDDDD] - Setby_JJJJJJJJJJ'.
            Thread [IIIIIIIIII] - Setby_DDDDDDDDDD'.
            Thread [GGGGGGGGGG] - Setby_IIIIIIIIII'.
            Thread [EEEEEEEEEE] - Setby_GGGGGGGGGG'.
Thread [EEEEEEEEEE] complete. Our 'threadlocal' string is now - Setby_GGGGGGGGGG'.
            Thread [HHHHHHHHHH] - Setby_GGGGGGGGGG'.
            Thread [BBBBBBBBBB] - Setby_GGGGGGGGGG'.
            Thread [FFFFFFFFFF] - Setby_GGGGGGGGGG'.
Thread [FFFFFFFFFF] complete. Our 'threadlocal' string is now - Setby_GGGGGGGGGG'.
...
            Thread [JJJJJJJJJJ] - Setby_GGGGGGGGGG'.
Thread [JJJJJJJJJJ] complete. Our 'threadlocal' string is now - Setby_GGGGGGGGGG'.
Thread [HHHHHHHHHH] complete. Our 'threadlocal' string is now - Setby_GGGGGGGGGG'.
            Thread [GGGGGGGGGG] - Setby_GGGGGGGGGG'.
Thread [GGGGGGGGGG] complete. Our 'threadlocal' string is now - Setby_GGGGGGGGGG'.
            Thread [IIIIIIIIII] - Setby_GGGGGGGGGG'.
            Thread [CCCCCCCCCC] - Setby_GGGGGGGGGG'.
Thread [CCCCCCCCCC] complete. Our 'threadlocal' string is now - Setby_GGGGGGGGGG'.
Thread [AAAAAAAAAA] complete. Our 'threadlocal' string is now - Setby_GGGGGGGGGG'.
            Thread [DDDDDDDDDD] - Setby_GGGGGGGGGG'.
Thread [DDDDDDDDDD] complete. Our 'threadlocal' string is now - Setby_GGGGGGGGGG'.
Thread [IIIIIIIIII] complete. Our 'threadlocal' string is now - Setby_GGGGGGGGGG'.
============== all threads complete.

I have not included the class which creates, starts, and joins the threads - if that's felt to be helpful, I'll gladly edit to add it.

Answer 1

Having separate instances doesn't help, as all instances the same static fields. Mutable statics are evil.

If you really can't change the class, you might instead just want to use a lock so each client gets to use the static fields one at a time. If you want to have different instances of the static fields, then you will probably need to play around with class loaders (the other obvious solution is to rewrite the bytecode, which is even less pleasant).

Answer 2

The ThreadLocal class does not change the behavior of the static field modifier. Static fields will continue to have one incarnation across multiple instances of the class, for they are created and initialized when the class is initialized.

To shed more light on this, ThreadLocal members are managed internally using a Map that is exists on a per-thread basis. The get() and set() calls operate on this Map. There is no 'magic' in ThreadLocal, whereby a static member loses its property of having a single incarnation. Static members when set as a ThreadLocal variable, are simply added to the Map for reference in another section of the code that will be executed by the thread. This does not prevent a second thread from obtaining a reference to the static member, and perform operations on the member field.

It is for this reason that Tom Hawtin's statements should be taken seriously - mutable statics simply do not constitute good design.

PS: Taking a look at the implementation of the Thread, ThreadLocal and ThreadLocal.ThreadLocalMap classes would be beneficial in clearing any misconceptions about the behavior of ThreadLocal objects.