Reputation: 7
I am new to threads and learning but in the code below why notify is not working. As per my understanding notify should execute the main thread and print the total when i=5. Please correct if i am wrong.
public class ThreadA {
public static void main(String[] args){
ThreadB b = new ThreadB();
b.start();
synchronized(b){
try{
System.out.println("Waiting for b to complete...");
b.wait();
}catch(InterruptedException e){
e.printStackTrace();
}
System.out.println("Total is: " + b.total);
}
}
}
class ThreadB extends Thread{
int total;
@Override
public void run(){
synchronized(this){
for(int i=0; i<10 ; i++){
total += i;
if(i==5){
System.out.println("Notify");
notify();
}
System.out.println("Total is in cal thread: "+i+":" + total);
}
}
}
}
Here is the output from the program:
Waiting for b to complete...
Total is in cal thread: 0:0
Total is in cal thread: 1:1
Total is in cal thread: 2:3
Total is in cal thread: 3:6
Total is in cal thread: 4:10
Notify
Total is in cal thread: 5:15
Total is in cal thread: 6:21
Total is in cal thread: 7:28
Total is in cal thread: 8:36
Total is in cal thread: 9:45
Total is: 45 –
Upvotes: 0
Views: 2162
Reputation: 182893
synchronized(b){
try{
System.out.println("Waiting for b to complete...");
b.wait();
Calling notify
affects only currently-waiting threads, a thread that begins waiting after the call will not be affected. Don't call wait
until you can 100% confirm, inside the synchronized block, that the thing you're waiting for hasn't already happened.
Upvotes: 3
Reputation: 96454
When you say:
As per my understanding notify should execute the main thread and print the total when i=5.
That's not how this works; it doesn't matter where in the synchronized block that you put the call to notify, the notify doesn't occur until the notifying thread releases the lock. ThreadB finishes executing the for loop before the main thread receives the notification. The thread receiving the notification can't act before that anyway, it needs to acquire the lock before it can leave the wait method. That is why all the "Total is in cal" lines print out before the "Total is" line.
If you want the "Total is" to show up earlier then you'd have to change ThreadB to release its lock sooner. You could move the synchronization to within the for loop so that it would let go of the lock after each increment.
The call to notify is actually redundant here, you can comment it out and the program still works.
What is going on here is that, since you're using the thread b as a lock, when that thread terminates it performs a notifyAll that is received by any threads waiting to acquire it. This is described in the api doc for Thread#join:
This implementation uses a loop of this.wait calls conditioned on this.isAlive. As a thread terminates the this.notifyAll method is invoked. It is recommended that applications not use wait, notify, or notifyAll on Thread instances.
If you run this version of the program that I changed to use a different lock that is not a thread, you can see there is no such implicit notification, and whether the program calls notify will actually matter. In this version commenting out the notify done by thread b will cause the program to hang.
public class ThreadA {
public static final Object lock = new Object();
public static void main(String[] args){
ThreadB b = new ThreadB();
b.start();
synchronized(lock){
try{
System.out.println("Waiting for b to complete...");
lock.wait();
}catch(InterruptedException e){
e.printStackTrace();
}
System.out.println("Total is: " + b.total);
}
System.out.println("done");
}
}
class ThreadB extends Thread{
int total;
@Override
public void run(){
synchronized(ThreadA.lock){
for(int i=0; i<10 ; i++){
total += i;
if(i==5){
System.out.println("Notify");
ThreadA.lock.notify(); // hangs if you comment this out
}
System.out.println("Total is in cal thread: "+i+":" + total);
}
}
}
}
As a rule you should call the wait inside of a loop, something like this:
synchronized(b) {
try{
// b.total needs to be volatile for this to work
while (b.total < 5) {
System.out.println("Waiting for b to complete...");
b.wait();
}
}catch(InterruptedException e){
e.printStackTrace();
}
That removes the order-dependence where you're assuming wait gets called before notify. That doesn't show up as an issue in your example, both because
a) the main thread has a huge head start (threadB has to start and get picked to run by the scheduler, while the main thread is already executing) and
b) the notifyAll is sent by the terminating thread that is being locked on.
In the real-world wrapping the wait in a loop is definitely a good idea, for removing order-dependence, for handling cases where another thread swoops in and changes something in between the time that the thread is notified and the time that it can reacquire the lock, and for handling spurious wakeups (where due to race conditions a thread can be woken up without having been notified).
Upvotes: 2
Reputation: 46891
In case of multi-threading you are not sure that b.wait()
in main
thread will be called before calling notify()
in ThreadB
. Sometime when b.start()
is called the run
method of ThreadB
is start executing before executing rest of statements of the main
thread.
If
wait
is called afternofity
thenmain
thread will wait for infinite time and in that case there is no one to notify it again.
If you want to finish the main
thread in the end after finishing all the activities by the another thread then you can use Thread.join() that waits for this thread to die. There is no need for wait
, notify
and synchronized
.
Sample code.
public class ThreadA {
public static void main(String[] args) {
ThreadB b = new ThreadB();
b.start();
try {
b.join();
} catch (InterruptedException e1) {
e1.printStackTrace();
}
System.out.println("Waiting for b to complete...");
System.out.println("Total is: " + b.total);
}
}
class ThreadB extends Thread {
int total;
@Override
public void run() {
for (int i = 0; i < 10; i++) {
total += i;
}
}
}
If you want to read more about Concurrency then find it here Guarded Blocks that explain it by creating a Producer-Consumer application.
Upvotes: 1