Thread safe global variable in Java

up vote2down votefavorite 1

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? java multithreading thread-safety share|improve this question asked May 3 at 14:32 user2219247 75110

Wasn't your intention to make long static in this example? Because in any other case there is no threadsafety issue. –  Michal Borek May 3 at 14:39  Make sure you notice my answer. Your MyThread class should not extend Thread and should implementRunnable. –  Gray May 3 at 14:48 add comment

2 Answers

activeoldestvotes

up vote5down voteaccepted

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. share|improve this answer answered May 3 at 14:37 yshavit 8,8251719

add comment

up vote2down vote

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 ofThreadSafe 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 didmyThread.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; } ... share|improve this answer edited May 3 at 14:52 Nathan Hughes 24.3k53370 answered May 3 at 14:37 Gray 47.4k659106

add comment