CODEWITHSUNDEEP
javamultithreading
jste89
jste89

Reputation: 446

Is it safe to store an Object in a hash within a static class, using the Thread Id to retrieve values for that specific thread?

I am making some changes to some code I have written to try and change it into a multi-threaded solution. Some of the elements from my main class were originally static, and have had to be changed as part of the changes I am making. I had the idea to store them in a HashMap, using the Id of the Thread as the key for retrieving the items - that way I could store a reference to the Runnable class in the hash and access the desired attributes for the given thread by using getters/setters. I defined the below code to do this:

import java.util.HashMap;

public class ThreadContext {

    private static HashMap<String, HashMap<String, Object>> tContext;

    static {
        initThreadContext();
    }

    public static void initThreadContext() {
        String id = String.valueOf(Thread.currentThread().getId());
        tContext  = new HashMap<>();
    }

    public static void setObject(String key, Object o) {
        String id = String.valueOf(Thread.currentThread().getId());
        HashMap<String, Object> hash = tContext.get(id);

        if( hash == null ) {
            hash = new HashMap<>();
            tContext.put(id, hash);     
        }

        hash.put(key, o);
    }

    public static Object getObject(String key) {
        String id = String.valueOf(Thread.currentThread().getId());
        HashMap<String, Object> hash = tContext.get(id);

        if( hash == null ) {
            hash = new HashMap<>();
            tContext.put(id, hash);
        }

        Object o = hash.get(key);

        return o;
    }
}

My question is: is it safe to do this, or should I try and find another way to do this? My example appears to work OK, but I'm unsure of any other side effects which may come about because of this.

EDIT: Example usage:

Foo foo = ((Foo)ThreadContext.getObject(Foo.CLASS_IDENTIFIER));

foo.doStuff();

Upvotes: 3

Views: 357

Answers (3)

user2023577
user2023577

Reputation: 2111

The ThreadLocal is certainly a better approach. But you want feedback on this code, so here it is.

The static block and the init can all be inlined on the static declaration.

You could use an IdentityHashMap and store thread instance themselves, avoiding the unclear risks around the thread id value stated above.

You could certainly use some static method synchronization for thread safety, but that would create contention. So a ConcurrentHashMap would locate the sub map for each thread, which in turn doesn't need synchronization (since only one thread could access it).

Regarding the safety (visibility to other unintended stackframes) when using a thread pool or executor and the likes, you can code yourself a try/finally or a closure (java87 lambda) to make sure you cleanup when you leave your code stackframes. No harder than the lock/unlock discipline.

BIG WARNING: if your code needing this custom threadlocal (or ANY thread local) will be inside a ForkJointTask.compute() subject to a ForkJoinPool during and calling a ForkJoinTask.join(), your thread will possibly run other identical ForkJoinTask.compute() (because of the thread continuation emulation) and your custom threadlocal could be initialized again and again (meaning, it will be clobbered) before even leaving the initial ForkJoinTask.compute(). This means you would need a stack of initial values managed in your try/finally... to tolerate re-entrance.

Upvotes: 1

Sathiesh
Sathiesh

Reputation: 419

Not sure what you are trying to do, however some of the points you should think are :

  • HashMap is not a synchronized object and has to be used in places where you don't need to worry about threads
  • In your case you seem to assume Thread Id will be unique which will not be when running on application servers. Some of the Application servers reuse thread ids and even use thread pool to reuse threads.

If you want to have data associated to a thread alone, use ThreadLocal. Again ThreadLocal should be used with Caution as there is no way JVM can clear the contents of ThreadLocal once your thread completes execution, if there is a thread pool. You will have to set the data and clear the data yourself.

Upvotes: 5

Bohemian
Bohemian

Reputation: 425083

There is already a way to do this using the JDK's ThreadLocal, which stores distinct references for each (local) thread.

Upvotes: 6

Related Questions