Notify not getting the thread out of wait state

Question

I am trying to use 2 threads. 1 thread prints only odd number and the other thread prints only even number and It has to be an alternative operation.

Eg:

Thread1 1
Thread2 2
Thread1 3
Thread2 4
and so on..

Below is the program, please let me know where I am going wrong as the thread1 is not coming out of wait state even when the thread2 is notifying it..

    public class ThreadInteraction {

    public static void main(String[] args) {
        new ThreadInteraction().test();
    }

    private void test() {
        ThreadA ta = new ThreadA();
        Thread t = new Thread(ta);
        t.start();

        try {
            Thread.sleep(1000);
        } catch (InterruptedException e1) {
            e1.printStackTrace();
        }

        for(int i=2;i<=50;){
            System.out.println("Thread2 "+i);
            synchronized (t) {
                try {
                    t.notify(); 
                    t.wait();
                } catch (Exception e) {
                    e.printStackTrace();
                }
            }
            i=i+2;
        }
    }
}

    class ThreadA implements Runnable{
        @Override
        public void run() {
            for(int i=1;i<50;){
                System.out.println("Thread1 "+i);
                synchronized (this) {
                        try {
                            notify();                           
                            wait();
                        } catch (InterruptedException e) {
                            e.printStackTrace();
                        }
                }
                i=i+2;
            }
        }
    }

Answer 1

You have a real confusion of threads and locks. I suggest you create one and only one object to use for locking to start with as you don't appear to have a clear idea what you are locking.

If you notify() and nothing is listening, the signal is lost. However, a wait() can wake spuriously.

For this reason, a notify() should be accompanied by a state change and a wait() should be in a loop checking that change.

Answer 2

Problem is that in one case you are taking lock on Thread t [synchronized (t) ] while in other case you are taking lock on TheadA object itself [synchronized(this)].

If you want threads to talk to each other then both should take lock on same object only then wait notify will work as you expect.

Edit:

There is another problem in your program, you are not using any variable to coordinate between 2 threads. SO you may see output like this 2,1,4,3...so on. Point is threads will work alternately but not in sequence. So you should share a single variable between 2 threads which should be incremented. Second issue is you are not taking care of spurious wake up calls [read some docs on this], you should always have wait called inside a while loop.

Answer 3

Modified my code based on the answer provided by Lokesh

public class ThreadInteraction {
    public static void main(String[] args) {
        new ThreadInteraction().test();
    }
    private void test() {
        ThreadA ta = new ThreadA();
        Thread t = new Thread(ta);
        t.start();

        try {
            Thread.sleep(1000);
        } catch (InterruptedException e1) {
            e1.printStackTrace();
        }

        for(int i=2;i<=50;){
            System.out.println("Thread2 "+i);
            synchronized (ta) {
                try {
                    ta.notify();    
                    ta.wait();
                } catch (Exception e) {
                    e.printStackTrace();
                }
            }
            i=i+2;
        }
    }
}
class ThreadA implements Runnable{
    @Override
    public void run() {
        for(int i=1;i<50;){
            System.out.println("Thread1 "+i);
            synchronized (this) {
                    try {
                        notify();                           
                        wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
            }
            i=i+2;
        }
    }
}