Shivam
Shivam

Reputation: 33

NotifyAll not working

This code is printing the Even/Odd numbers from two different threads. Here my programme stucks in the wait() and unable to wake up the slept thread using notifyAll().

Want to know, Why notifyAll is unable to wake up all the slept threads? Need to know what I am doing wrong here.

class EvenOddThread {
  public static void main (String [] args) {
  Runnable runnEven = new ThreadPrint (true, 10);
  Runnable runnOdd = new ThreadPrint (false, 10);

  Thread t1 = new Thread (runnEven);
  Thread t2 = new Thread (runnOdd);

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

class ThreadPrint implements Runnable {
  private boolean isEvenPrint = false;
  private int maxPoint = 0;
  ThreadPrint (boolean isEven, int max) {
    isEvenPrint = isEven;
    maxPoint = max;
  }

  @Override
  public void run () {
    Print p = new Print();
    if (isEvenPrint)
      p.printEven(maxPoint);
    else
      p.printOdd(maxPoint);
  }
}

class Print {


  public synchronized void printEven (int maxPoint) {
    int even = 0;
    while (even <= maxPoint) {
      System.out.println(even);
      even = even + 2;
      try {
      wait();
    }
    catch (Exception e) {
      System.out.println(e);
    }

    notifyAll();
    }
  }

  public synchronized void printOdd (int maxPoint) {
    int odd = 1;
    while (odd <= maxPoint) {


    System.out.println(odd);
    odd = odd + 2;
    try {
      wait();
    }
    catch (Exception e) {
      System.out.println(e);
    }

      notifyAll();
    }
  }
}

Thanks, Shivam

Upvotes: 1

Views: 286

Answers (4)

Shivam
Shivam

Reputation: 33

class EvenOddThread {
  public static void main (String [] args) {
  Runnable runnEven = new ThreadPrint (10);

  Thread t1 = new Thread (runnEven);
  Thread t2 = new Thread (runnEven);

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

  }
}

class ThreadPrint implements Runnable {
  private int maxPoint = 0;
  private int counter = 0;
  ThreadPrint (int max) {
    maxPoint = max;
  }

  @Override
  public void run () {
    while (counter <= maxPoint) {
      synchronized(this) {
        if (counter % 2 == 0) {

          System.out.println("By Thread" + Thread.currentThread().getName() +" :: "+ counter);
          counter ++;
          notify();
          try {
            wait();
          }
          catch (Exception e) {
            System.out.println(e);
          }
        }
        else if (counter % 2 != 0){
          System.out.println("By Thread" + Thread.currentThread().getName() +" :: "+ counter);
          counter ++;
          notify();
          try {
            wait();
          }
          catch (Exception e) {
            System.out.println(e);
          }
        }
      }
    }
  }
}

Thanks, guys for your answers but I took help from here to figure out what should I do to correct my code.

Now, It's working the way I want. :)

Upvotes: 0

St.Antario
St.Antario

Reputation: 27385

Seems like you just want two threads print odd/even number in a ping-pong style. Then you can make use of ReentrantLock. There are several of ways to do this.

I. Locks

public static final ReentrantLock lock = new ReentrantLock();
public static final int LIMIT = 50;
public static int toPrint = 0;

public static void main(String[] args){
    Thread t1 = new Thread(() -> {
        while (true){
            try {
                lock.lock();
                if(toPrint > LIMIT) break;
                else if(toPrint % 2 == 0) {
                    System.out.println(toPrint++);
                }
            } finally {
                lock.unlock();
            }
        }
    });

    Thread t2 = new Thread(() -> {
        while (true) {
            try {
                lock.lock();
                if (toPrint > LIMIT) break;
                else if (toPrint % 2 == 1) {
                    System.out.println(toPrint++);
                }
            } finally {
                lock.unlock();
            }
        }
    });
    t1.start();
    t2.start();
}

II. Volatile (fragile, but works)

public static final int LIMIT = 50;
public static volatile int toPrint = 0;

public static void main(String[] args){
    Thread t1 = new Thread(() -> {
        while (true){
            int cur = toPrint;
            if(cur > LIMIT) break;
            else if(cur % 2 == 0){
                System.out.println(cur);
                toPrint = cur + 1;
            }
        }
    });

    Thread t2 = new Thread(() -> {
        while (true) {
            int cur = toPrint;
            if(cur > LIMIT) break;
            else if(cur % 2 == 1){
                System.out.println(cur);
                toPrint = cur + 1;
            }
        }
    });
    t1.start();
    t2.start();
}

Upvotes: 1

Jomu
Jomu

Reputation: 349

notifyAll() should notify all threads waiting on specific object, not all threads waiting anywhere. What you do, you make an instance of Print for each thread - and then use unqualified wait(), notifyAll() - meaning your threads communicate using variables local to each one of them.

So first you have to make them use same synchronization/signaling object(s).

Second, you need to provide some initial kick, at least. Be careful here, as you need to make this signal when you are sure at least one of them is already listening. Best case is to ensure both of them are listening, so you can escape zillions of possible race condition situations.

=rant= Sun made big mess here, IMO, by hiding (mutex, condition, signal) triad from programmers - most probably in a bid to make multithreading simpler. They failed. =/rant=

Upvotes: 1

jspcal
jspcal

Reputation: 51904

Both threads enter wait after printing the first number. Since both threads are paused, neither thread can wake up the other.

In order to have a pair of threads alternate in this way, you could have separate conditions that control the progress of each thread. Each thread would unblock the other thread after completing its own unit of work. This can be done with a Semaphore, CyclicBarrier, or Phaser.

Upvotes: 3

Related Questions