synchronized block for an Integer object

Question

I just came across the synchronized block in Java and wrote a small programm to test how it works.

I create 10 threads and let each thread increment an Integer object 1000 times.

So with synchronization I would assume a result of 10000 after all threads have finished their work and a result of less than 10000 without synchronization .

However the synchronization is not wokring as I expected.

I guess it has something to do with immutability of the object or so.

My program:

public class SyncTest extends Thread{

    private static Integer syncObj = new Integer(0);
    private static SyncTest[] threads = new SyncTest[10];

    private boolean done = false;

    public void run(){
        for(int i = 0; i < 1000; i++){
            synchronized(syncObj){
                syncObj ++;
            }
        }
        done = true;
    }

    public static void main(String[] args) {

        for(int i=0; i < threads.length; i++){
            threads[i] = new SyncTest();
            threads[i].start();
        }

        while(!allDone()); //wait until all threads finished

        System.out.println(syncObj);
    }

    private static boolean allDone(){
        boolean done = true;
        for(int i = 0; i < threads.length; i++){
            done &= threads[i].done; 
        }

        return done;
    }
}

Can someone clarify this?

*However the synchronization is not wokring as expected.* - what is happening that is unexpected? — OldCurmudgeon, Apr 29 '13 at 14:08
The answer in this [post][1] explains it pretty well. Go check it out ;). [1]: http://stackoverflow.com/questions/9278764/are-primitive-datatypes-thread-safe-in-java — MMeersseman, Apr 29 '13 at 14:10
@OldCurmudgeon I was expecting that I get the 10000 but I was not. The Output was always something lower than 10000. But now I know why ;) — das Keks, Apr 29 '13 at 14:18
@jsn `volatile` isn't required as `done` is not being changed by other threads. — Ravi K Thapliyal, Apr 29 '13 at 15:00
@Ravi, you are right. It is not changed outside the thread. I am still concerned that the main thread might cache each of them as false. — jn1kk, Apr 29 '13 at 15:13
@jsn you are right, the main thread may not pick up the value being set to true. The OP should use threads[i].join() — rolfl, Apr 29 '13 at 15:17
@rolfl My running seems to work but I didn't notice the join method before. Either way I prefere the join method. Thanks :) — das Keks, Apr 29 '13 at 15:32
@who ever voted down: What is the problem with my question and how can I improve it so that it may can help ohters? — das Keks, Apr 29 '13 at 15:33
@rolfl Since there's nothing else that keeps the threads occupied after `done = true;` I doubt that the main thread would somehow miss the sync. But, yes using `thread.join()` is better any day. — Ravi K Thapliyal, Apr 29 '13 at 15:52
It's not just the cache part, it's the tight loop (100%CPU) while waiting — rolfl, Apr 29 '13 at 15:55

rolfl · Accepted Answer · 2013-04-29T15:19:29.080

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'.

score 2 · Answer 2 · answered Apr 29 '13 at 14:12

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 falses

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.

score 1 · Answer 3 · answered Apr 29 '13 at 14:07

1

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.

answered Apr 29 '13 at 14:07

flavian

28,161
11
65
105

1

OP is experimenting with `synchronized(`. – OldCurmudgeon Apr 29 '13 at 14:09

synchronized block for an Integer object

3 Answers3

Linked

Related