multithreading - Inconsistent results with java threads

Question

I have a thread class which implements runnable and an int counter as instance variable. Two synchronized methods add and sub. When I run my test class somehow it is printing wrong results once in couple of times. As far as I understand when a method is synchronized, entire object will be locked for accessing by other threads, with this logic every time we should get same results right? Some how that is not the case. Am I missing something?

My machine is Windows 7, 64 bit.

 public class ThreadClass implements Runnable {

        int counter = 0;

        @Override
        public void run() {
            add();
            sub();
        }

        public synchronized void add() {
            System.out.println("ADD counter" + (counter = counter + 1));
        }

        public synchronized void sub() {
            System.out.println("SUB counter" + (counter = counter - 1));
        }
    }

Testclass

public class ThreadTest {

    public static void main(String args[]) {
        ThreadClass tc = new ThreadClass();
        Thread tc0 = new Thread(tc);
        tc0.start();
        tc0.setPriority(Thread.MAX_PRIORITY);
        Thread tc1 = new Thread(tc);
        tc1.start();
        tc1.setPriority(Thread.NORM_PRIORITY);
        Thread tc2 = new Thread(tc);
        tc2.start();
        tc2.setPriority(Thread.MIN_PRIORITY);
    }
}

Results

ADD counter1
ADD counter2
SUB counter1
SUB counter0
ADD counter1
SUB counter0

Note: You may need to do couple of runs to produce this inconsistency.

Answer 1

Your results look correct.

During the execution of the methods, an exclusive lock on the object is obtained, but between the add() and sub() calls, the threads can freely interleave.

If you end up with a total of 0 after all the threads have run, then none of them overwrote eathother and the access to counter was synchronized.

If you wish to have counter only go from 0 to 1 sequentially and never hit 2, then do the following (which will render the method-level synchronization redundant so long as no other classes are involved):

@Override
public void run() {
    synchronize(this) {
        add();
        sub();
    }
}

However, this makes the whole point of the threads useless since you could do that in a single-threaded loop.