Concurrency in Java using synchronized blocks that do not produce the expected results

Question

The following is a trivial java program. It has a counter called "cnt" that increments and then is added to a list called "monitor". "cnt" is incremented by multiple threads, and values ​​are added to the "monitor" by multiple threads.

At the end of the "go ()" method, cnt and monitor.size () should have the same value, but they do not. monitor.size () really has the correct value.

If you change the code by uncommenting one of the commented synchronized blocks and commenting on the currently inconsistent, the code gives the expected results. In addition, if you set the number of threads (THREAD_COUNT) to 1, the code will produce the expected results.

This can only be reproduced on a machine with several real cores.

public class ThreadTester { private List<Integer> monitor = new ArrayList<Integer>(); private Integer cnt = 0; private static final int NUM_EVENTS = 2313; private final int THREAD_COUNT = 13; public ThreadTester() { } public void go() { Runnable r = new Runnable() { @Override public void run() { for (int ii=0; ii<NUM_EVENTS; ++ii) { synchronized( monitor) { synchronized(cnt) { // <-- is this synchronized necessary? monitor.add(cnt); } // synchronized(cnt) { // cnt++; // <-- why does moving the synchronized block to here result in the correct value for cnt? // } } synchronized(cnt) { cnt++; // <-- why does moving the synchronized block here result in cnt being wrong? } } // synchronized(cnt) { // cnt += NUM_EVENTS; // <-- moving the synchronized block here results in the correct value for cnt, no surprise // } } }; Thread[] threads = new Thread[THREAD_COUNT]; for (int ii=0; ii<THREAD_COUNT; ++ii) { threads[ii] = new Thread(r); } for (int ii=0; ii<THREAD_COUNT; ++ii) { threads[ii].start(); } for (int ii=0; ii<THREAD_COUNT; ++ii) { try { threads[ii].join(); } catch (InterruptedException e) { } } System.out.println("Both values should be: " + NUM_EVENTS*THREAD_COUNT); synchronized (monitor) { System.out.println("monitor.size() " + monitor.size()); } synchronized (cnt) { System.out.println("cnt " + cnt); } } public static void main(String[] args) { ThreadTester t = new ThreadTester(); t.go(); System.out.println("DONE"); } } 

Answer 1

Ok let's look at the various features you mentioned:

1.

 for (int ii=0; ii<NUM_EVENTS; ++ii) { synchronized( monitor) { synchronized(cnt) { // <-- is this synchronized necessary? monitor.add(cnt); } synchronized(cnt) { cnt++; // <-- why does moving the synchronized block to here result in the correct value for cnt? } } 

First, the monitor object is distributed between threads, so getting a lock on it (i.e. synchronization) ensures that the code inside the block will be executed only by one thread at a time. Thus, 2 synchronized inside the external are not needed, the code is protected in any case.

2.

 for (int ii=0; ii<NUM_EVENTS; ++ii) { synchronized( monitor) { monitor.add(cnt); } synchronized(cnt) { cnt++; // <-- why does moving the synchronized block here result in cnt being wrong? } } 

Okay, this is a little complicated. cnt is an Integer object, and Java does not allow the Integer object to be modified (integers are immutable), although the code assumes that this is what happens here. But what will happen is that cnt ++ will create a new Integer with a value of cnt + 1 and override cnt. This is what the code actually does:

 synchronized(cnt) { Integer tmp = new Integer(cnt + 1); cnt = tmp; } 

The problem is that although one thread will create a new cnt object, all other threads are waiting to get a lock on the old one. Now the thread releases the old cnt and then tries to get the lock of the new cnt object and get it, and the other thread gets the lock of the old cnt object. Suddenly, in a critical section, there are 2 threads executing the same code and causing a race condition. This is where the wrong results happen.

If you delete the first synchronized block (the one with the monitor), then your result will become even more incorrect, because the chances of a race increase.

In general, you should try to use synchronization only for the final variables so that this does not happen.