Synchronized() block is not working as expected

Question

private static Integer balance=0;

public static void deposit(final int amt) {
    Thread t = new Thread(new Runnable() {
        public void run() {
            synchronized(balance) {
                System.out.println("Balance at start is: "+balance);        
                balance+=amt;
                System.out.println("deposited " + Integer.toString(amt) + " to funds. Now at " + Integer.toString(balance));
            }
        }
    });
}

When I run the above simple deposit function, I expect that two threads should not enter at same time in synchronized block. But with operation Sequence as below:

1. Depo100
2. Depo200
3. Depo700

Output is as below:

------------------
Balance at start is: 0
deposited 100 to funds. Now at 100
Balance at start is: 100
Balance at start is: 100
deposited 700 to funds. Now at 800
deposited 200 to funds. Now at 1000

As we can see two threads entered at same time in the synchronized block and accessed balance object which is NOT expected. What I am doing wrong here ? I am newbie for Multithreading. Thanks in advance.

Answer 1

Integer (like all primitive wrapper classes) is immutable. Every time you "add a number to it", you're actually setting the field to a new instance:

balance += amt;

is actually evaluated as follows:

balance = Integer.valueOf(balance.intValue() + amt);

So you're synchronizing on different objects each time (unless amt happens to be zero, and balance.intValue() is in the range cached by your JVM's implementation of Integer.valueOf).

You need a fixed value that you can synchronize on. You can use a fixed reference with a mutable value, for example an Integer[] with length 1, or an AtomicInteger (although synchronizing on an AtomicInteger always feels a bit wrong to me - but actually, you don't need to use synchronization because you can use AtomicInteger.getAndIncrement or AtomicInteger.incrementAndGet).

---

Note that you should probably make the reference to the thing you synchronize on final, to avoid accidentally setting it to some different value and breaking the mutual exclusion again.

See this question for more details.