Thread safe global variable in Java

Question

I am trying to understand the thread safety mechanism in java and I need some help. I have a class:

public class ThreadSafe {

    private Executor executor = new ScheduledThreadPoolExecutor(5);

    private long value = 0;

    public void method() {
        synchronized (this) {
            System.out.println(Thread.currentThread());
            this.value++;
        }
    }

    private synchronized long getValue() {
        return this.value;
    }

    public static void main(String... args) {
        ThreadSafe threadSafe = new ThreadSafe();
        for (int i = 0; i < 10; i++) {
            threadSafe.executor.execute(new MyThread());
        }

    }

    private static class MyThread extends Thread {

        private ThreadSafe threadSafe = new ThreadSafe();

        private AtomicBoolean shutdownInitialized = new AtomicBoolean(false);

        @Override
        public void run() {
            while (!shutdownInitialized.get()) {
                threadSafe.method();
                System.out.println(threadSafe.getValue());
            }
        }
    }

}

Here I am trying to make the value thread safe, to be accessed only by one thread at a time. When I am running this program I see that there is more than one thread operating on the value even if I wrap it within the synchronized block. Of course this loop will be infinite but its just an example, I am stopping this program manually after few seconds, so I have:

2470
Thread[pool-1-thread-3,5,main]
2470
Thread[pool-1-thread-5,5,main]
2470
Thread[pool-1-thread-2,5,main]

Different threads are accessing and changing this value. Can somebody explain to me why is this so? And how to make this global variable thread safe?

Answer 1

Each thread has its own ThreadSafe, and each ThreadSafe has its own, different value. Moreover, synchronized methods lock on this, so each ThreadSafe is locking on itself -- and none of them are being shared among threads. This is called thread-locality, and it's the easiest way to ensure thread safety. :)

To get at the experiment that I think you want, you'd need to change MyThread such that its constructor takes a ThreadSafe argument (instead of constructing one). Then, have the main method create one ThreadSafe and give it to each MyThread at construction time.

Answer 2

You are getting the same value every time because each of your Runnables has its own instance of the ThreadSafe class.

If you want them all to share the same class, then you are going to need to only have one instance of ThreadSafe and pass it into all of your jobs -- see below. As mentioned, an AtomicLong is the way to go if you want a thread-safe shared long.

Also, your MyThread class should not extend Thread. It should instead implements Runnable. Your code is working because Thread already implements Runnable. If you did myThread.interrupt() it would not actually be interrupting the thread because it is the thread-pool threads that are calling your run() method.

Something like the following would work:

ThreadSafe threadSafe = new ThreadSafe();
for (int i = 0; i < 10; i++) {
    threadSafe.executor.execute(new MyRunnable(threadSafe));
}
...
private static class MyRunnable implements Runnable {
    private final ThreadSafe threadSafe;
    public MyRunnable(ThreadSafe threadSafe) {
       this.threadSafe = threadSafe;
    }
    ...