0

I have below program having 2 threads T1 and T2. T1 is running first and goes into waiting state. T2 is calling notify. Why T1 thread doesn't resumes and prints "Thread-0 is waken up"

public class WaitNotifyTest{
    public static void main(String[] args) {
        Thread t1 = new Thread(new Runnable() {
            @Override
            public void run() {
                synchronized (this) {
                    System.out.println(Thread.currentThread().getName() + " is running");
                    try {
                        wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    System.out.println(Thread.currentThread().getName() + " is waken up");
                }
            }
        });
        Thread t2 = new Thread(new Runnable() {
            @Override
            public void run() {
                synchronized (this) {
                    try {
                        Thread.sleep(3000);
                        System.out.println(Thread.currentThread().getName() + " is running");
                        notify();
                        System.out.println(Thread.currentThread().getName() + " notifying");
                    } catch (InterruptedException e1) {
                        e1.printStackTrace();
                    }
                }
            }
        });
        t1.start();
        t2.start();
    }
}

Output comes as

Thread-0 is running
Thread-1 is running
Thread-1 notifying

Note after this output my program doesn't ends/terminate. Can anyone please explain why my program not getting terminate.

Pradeep Singh
  • 1,094
  • 1
  • 9
  • 24
  • You might need `notifyAll()` instead of notify [notifyAll vs notify](https://stackoverflow.com/questions/37026/java-notify-vs-notifyall-all-over-again) – XPLOT1ON Jun 09 '17 at 08:59
  • 1
    Nope. I have tried this. It gives the same result. nothing gets change. – Pradeep Singh Jun 09 '17 at 09:00

6 Answers6

2

two issue is there in your code 1.You should use same object to communicate between the thread. 2.call wait and notify on same object on which you have taken the lock.

public class WaitNotifyTest{


    public static void main(String[] args) {
        Object lock=new Object(); 
            Thread t1 = new Thread(new Runnable() {
                @Override
                public void run() {
                    synchronized (lock) {
                        System.out.println(Thread.currentThread().getName() + " is running");
                        try {
                            lock.wait();
                        } catch (InterruptedException e) {
                            e.printStackTrace();
                        }
                        System.out.println(Thread.currentThread().getName() + " is waken up");
                    }
                }
            });
            Thread t2 = new Thread(new Runnable() {
                @Override
                public void run() {
                    synchronized (lock) {
                        try {
                            Thread.sleep(3000);
                            System.out.println(Thread.currentThread().getName() + " is running");
                            lock.notify();
                            System.out.println(Thread.currentThread().getName() + " notifying");
                        } catch (InterruptedException e1) {
                            e1.printStackTrace();
                        }
                    }
                }
            });
            t1.start();
            t2.start();
        }
    }
gati sahu
  • 2,576
  • 2
  • 10
  • 16
1

I think it is due to the synchronized (this) being used for locking purpose.

In t1, this would refer to the Object of anonymous class created there.

In t2, this would refer to the Object of another anonymous class created there. In simple words, both refer to different objects.

For, wait-notify to work, you must have lock on same object.

gprathour
  • 14,813
  • 5
  • 66
  • 90
  • 1
    I was testing it and debugging. In both the threads this refer to the Object of WaitNotifyTest class. – Pradeep Singh Jun 09 '17 at 09:06
  • 1
    @PradeepSingh No brother, they are referring to two different objects. As in the other comment you have written `org.A.WaitNotifyTest$1@2b4ebf39 ` and in other `org.A.WaitNotifyTest$2@57c44c58`. Notice Test$1 and Test$2. These two are different objects. – gprathour Jun 09 '17 at 09:10
  • 2
    @gprathour is right. They refer to two different instances of `Runnable`. After all, you are using `this` inside two different objects you just created using `new`. How can they be the same? – Davide Spataro Jun 09 '17 at 09:16
1

The problem is you are not synchronizing on the same object. Try printing this in each of the Threads to verify. Then try synchronizing on the same object, call wait and notify on that lock (otherwise you won't get the right response).

   public class WaitNotifyTest{
    public static void main(String[] args) {
        Integer someObject = 2;
        Thread t1 = new Thread(new Runnable() {
            @Override
            public void run() {
                System.out.println("t1 this = " + this);
                synchronized (someObject) {
                    System.out.println(Thread.currentThread().getName() + " is running");
                    try {
                        someObject.wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    System.out.println(Thread.currentThread().getName() + " is waken up");
                }
            }
        });
        Thread t2 = new Thread(new Runnable() {
            @Override
            public void run() {
                System.out.println("t2 this = " + this);
                synchronized (someObject) {
                    try {
                        Thread.sleep(3000);
                        System.out.println(Thread.currentThread().getName() + " is running");
                        someObject.notify();
                        System.out.println(Thread.currentThread().getName() + " notifying");
                    } catch (InterruptedException e1) {
                        e1.printStackTrace();
                    }
                }
            }
        });
        t1.start();
        t2.start();
    }
}
1

You are not using wait and notify correctly.

You must synchronize on the same object in order the threads to be able to wait-notify each others. You can share an object among them and call waitand notify withing a synchronized block using the same shared Object as monitor. The following fixes the problem:

public static void main(String[] args) {
    Object obj = new Object();

    Thread t1 = new Thread(new Runnable() {
        @Override
        public void run() {
            synchronized (obj) {
                System.out.println(Thread.currentThread().getName() + " is running");
                try {
                    obj.wait();
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
                System.out.println(Thread.currentThread().getName() + " is waken up");
            }
        }
    });
    Thread t2 = new Thread(new Runnable() {
        @Override
        public void run() {
            synchronized (obj) {
                try {
                    Thread.sleep(3000);
                    System.out.println(Thread.currentThread().getName() + " is running");
                    obj.notify();
                    System.out.println(Thread.currentThread().getName() + " notifying");
                } catch (InterruptedException e1) {
                    e1.printStackTrace();
                }
            }
        }
    });
    t1.start();
    t2.start();
}

Note that synchronized (this) is now synchronized (obj) and wait and wait and notify are not obj.wait() and obj.notify().

Davide Spataro
  • 7,319
  • 1
  • 24
  • 36
1

I agree with the other answers that you are synchronizing over the wrong object.

I suggest to "think in monitors". Monitors are objects that restrict concurrent access to preserve internal cosistent state. So the synchronization is clearly ensured by the monitor and is not spread all over the place in any single thread. Threads should not synchronize each other they should be synchronized by the resource they try to access.

public class Main {


    private static class Monitor {

        public synchronized void operation1() {
            System.out.println(Thread.currentThread().getName() + " is running");
            try {
                wait();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            System.out.println(Thread.currentThread().getName() + " is waken up");
        }


        public synchronized void operation2() {
            try {
                Thread.sleep(3000);
                System.out.println(Thread.currentThread().getName() + " is running");
                notify();
                System.out.println(Thread.currentThread().getName() + " notifying");
            } catch (InterruptedException e1) {
                e1.printStackTrace();
            }
        }

    }


    public static void main(String[] args) {

        Monitor monitor = new Monitor();

        Thread t1 = new Thread(new Runnable() {
            @Override
            public void run() {
                monitor.operation1();
            }
        });

        Thread t2 = new Thread(new Runnable() {
            @Override
            public void run() {
                monitor.operation2();
            }
        });

        t1.start();
        t2.start();

    }

}
oopexpert
  • 767
  • 7
  • 12
1

In addition to the fact ( mentioned by others) that you lock on different objects, you have another issue in your code. You use Thread.sleep to make sure that the waiting thread starts waiting before the other calls notify, but you still don't have a gueranty that it won't happen the other way around, making the waiting thread wait forever.

You have to also use some 'condition variable' in a while loop to be on the safe side.

Read oracle's docs for examples and further explanation.

Elist
  • 5,313
  • 3
  • 35
  • 73
  • 1
    Yes initially i used some variable and condition to maintain the order of execution of 2 threads. But to make the problem simpler and shorter. i used Thread.sleep. Thanks for sharing knowledge. – Pradeep Singh Jun 09 '17 at 09:53