synchronized method while using wait()

Question

I ran the following code:

class Counter extends Thread {

 static int i=0;
//method where the thread execution will start
public void run(){
    //logic to execute in a thread

    while (true) {
        increment();
    }
}

public synchronized void increment()  {
    try {
        System.out.println(this.getName() + " " +  i++);
        wait(1000);
        notify();
    } catch (InterruptedException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }

}
//let’s see how to start the threads
public static void main(String[] args){
    Counter c1 = new Counter();
    Counter c2 = new Counter();
    c1.setName("Thread1");
    c2.setName("Thread2");
    c1.start();
    c2.start();
}
}

The result of this code was (added line numbers):

1: Thread1 0
2: Thread2 1
3: Thread2 2
4: Thread1 3
5: Thread2 4
6: Thread1 4
7: Thread1 5
8: Thread2 6
stopping...

Since increment method is synchronized and since it contains wait(1000) I didnt expect: 1. Thread2 to print 2 consecutive prints: lines 2,3 I expected the threads to interleave their prints 2. on lines 5,6 the i remains 4.

could anyone give me an explanation for this?

Answer 1

Synchronized instance methods like this:

public synchronized void foo() { 
    ... 
}

are roughly equivalent to:

public void foo() {
   synchronized(this) {
       ...
   }
}

Do you see the problem here? The synchronization is done on the current instance.

Since you are creating two separate Thread objects each increment method will synchronize on a different object, thus rendering the lock useless.

You should either make your increment method static (thus the locking is done on the class itself) or use a static lock object:

private static final Object locker = new Object();

public void foo() {
   synchronized(locker) {
       ...
   }
}

And one final advice: the preferred way to create a thread in java is by implementing Runnable, not extending Thread.

Answer 2

You are only synchronizing at the instance level. To synchronize across all Counter instances you need the increment method to be static as well as synchronized.

As it stands all your threads run freely, concurrent with each other, because they share no synchronization mechanism.