synchronized block for an Integer object
syncObject is changing each time you ++ it (the ++ is converting it to a primitive int, incrementing it, and then autoboxing it back to the Integer object. Integer objects are immutable ... once they are created, they cannot change.
Bottom ine is that you are not using the same syncPObj in all the threads, different threads use different syncObjects at different times to sync on.
use one object as the synchronization (call it syncObj), and declare it as a final Object:
private static final Object syncObject = new Object();
Then your counter should be a primitive (int) for perofrmance, call it 'counter' or something.
Synchronize on syncObject, and increment counter.
Edit: as per @jsn, the done flag is also broken in that your code has a 'tight loop' on the isAllDone() method, and that is bad practice. You should use thread[i].join() to wait (blocking) on each thread's completion, and then check the status from that. Using an ExecutorService is the 'right way'.
As assumed it is because of the immutability of the Integer object.
I've changed the synchonized block to
Integer old = syncObj;
syncObj ++;
System.out.println(syncObj == old);
and my console gets filled with false
s
So each time I increment the Integer
a new object is createt.
Therefore I only read from the old Object and it will not be locked.
These operations are usually done with Atomic
. Have a look here. These structures are specifically designed for multi-threaded computation. Normal implementations are not thread safe.