Notify and wait code problem

[Aditya Soni]

public class NotifyAndWaitExample2 {
	static int i=0;

	public static void main(String[] args) {

			Thread t1= new Thread(new Runnable(){

			@Override
			public void run() {
			synchronized (this) {
			if(i<=0){
				System.out.println("i="+i+"in t1");
				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 t4= new Thread(new Runnable() {
			
			@Override
			public void run() {
				synchronized (this) {
				i++;	
				System.out.println("i="+i+"in t4");
				System.out.println(Thread.currentThread().getName()+"is notifying");
				try {
					Thread.sleep(1000);
				notify();
				System.out.println("notified");
				} catch (InterruptedException e1) {
					// TODO Auto-generated catch block
					e1.printStackTrace();
				}
}
			}		});
		
		
		
		t1.start();
t4.start();
}

}
}
				

Here It is showing ouptut:-
i=0in t1
i=1in t4
Thread-0is running
Thread-1is notifying
notified

but why after printing "notified" it doesn't loose lock to thread "t1 run() method " and continue with the code after wait() in t1 . i.e it should print "Thread-0is waken up" after printing "notified".

[VarunS Singh]

It is because you have used synchronized block for each thread.
No other thread will wake up and start executing unless the current thread has done execution in its synchronized block.

Please correct me If I am Wrong.

[Aditya Soni]

so you mean to say calling notify() method won't release lock.?

correct me if am wrong!

[Stephan van Hulst]

Calling notify() will wake up one thread that has called wait() on the same object you called notify() on.

Sadly, you didn't call wait() and notify() on the same object. Hint: in this case the type of your lock object is Runnable, not NotifyAndWaitExample2.

A few other remarks:

You should always call wait() inside a loop that checks a condition. wait() is allowed to return without any thread having called notify() or notifyAll(), so you need to communicate between the threads using a condition.

Don't use synchronized in the method declaration, and don't synchronize on this. 9 out of 10 times, your lock should be owned by the object you want to synchronize, and it shouldn't be available to the outside world. That means you should do something like this:

final class ThreadSafe {

  private final Object lock = new Object();

  private boolean threadTwoHasFinishedSleeping;

  ...
}

However, since Java released the concurrency framework, you wouldn't use synchronized blocks anymore, and prefer Lock instead:

final class ThreadSafe {

  private final Lock lock = new ReentrantLock();

  private final Condition threadTwoHasFinishedSleepingCondition = lock.newCondition();

  private boolean threadTwoHasFinishedSleeping;

  ...
}

[Aditya Soni]

Thanks for replying,

So there are two more questions arise on mind after reading your answer i.e.

1) Why we can't we use "this" instead of private "lock" reference ??
2) And is calling notify will release the lock also? so as another thread will accquire the lock and continue its work?


hope you

[VarunS Singh]

Have a look. Here what the problem is when notify is called in the synchronized method of thread t4, dont you think you will be needing to rerun the t1 thread again.
Thats what I did here, add t1.run() in your code and see if it helps

try {
                    Thread.sleep(2000);
                
                notify();
                System.out.println("notified");
                t1.run();
                
                
                
                
                } catch (Exception e1) {
                    // TODO Auto-generated catch block
                    System.out.println(e1);
                }

[Stephan van Hulst]

No, that is downright wrong. That will just cause the second thread to run the code that was initially given to the first thread to run, making it run twice, and deadlocking the program because both threads will be stuck waiting.

[Tobias Bachert]

Synchronizing on this can be useful (see e.g. Collections#synchronizedXXX), but exposing the used lock object (which is done by locking on this) provides a way to block the method(s) in a malicious way (on purpose or accidentally), i.e. the following code would block the run method forever once it acquires the lock.

final Runnable objectThatLocksOnThis = new Runnable() {
    public synchronized void run() { /* Do something */ }
}
...
synchronized (objectThatLocksOnThis) {
    for (;;) {
        LockSupport.park();
        Thread.interrupted();
    }
}

Notify will not release the lock, the notified thread(s) have to wait until the caller of the #notify method releases the objects monitor.

[Stephan van Hulst]

Aditya Soni wrote:1) Why we can't we use "this" instead of private "lock" reference ??

Because this refers to different objects. For the first thread, this refers to the Runnable that you passed to the first thread, and for the second thread this refers to the Runnable that you passed to the second thread.

That means the first thread is waiting on a different object that the object that the second thread notified.

Explain to us what object you wanted to wait on when you called wait() and notify().

2) And is calling notify will release the lock also? so as another thread will accquire the lock and continue its work?

There is no one lock. You're using two locks.

[Henry Wong]

Stephan van Hulst wrote:
There is no one lock. You're using two locks.

As an aside, another issue is that the notification is done in the blind. There is no state, and hence, no checking if waiting is even needed.

This means, even if the OP fixed the issues, and use a single shared lock... there is still a race condition, where the t4 thread gets the lock first, and sends the notification. This notification would be lost, and since there is no state, there is no way that the t1 thread would even know that the t4 thread already executed, and simply waits for a notification (that has already been sent and discarded).

Henry