What is the good solution for calling notifyAll?

Question

I have a litle question about wait() and notifyAll() methods. The code is simulating the 'Race' of two threads.

Let`s look at the code - the problem is that notifyAll() method do nothing with waiting threads cause the main method get the lock first... The simple solve is to set some delay (see the commented line). But it is a bad practice. What is the good solution of this problem? I wish to use only wait/notifyAll/join methods.

public class TestThreads {
  public static void main(String[] args) throws InterruptedException {
    System.out.println("_start main");
    Object lock = new Object();
    Thread t1 = new Thread(new Car("Red car", lock));
    Thread t2 = new Thread(new Car("Black car", lock));
    t1.start();
    t2.start();
    //Thread.sleep(10L);
    synchronized (lock){
      System.out.println("Let`s go!");
      lock.notifyAll();
    }
    t1.join();
    t2.join();
    System.out.println("_exiting from main...");
  }
}

class Car implements Runnable {
  private final String name;
  private final Object lock;

  public Car(String name, Object lock) {
    this.name = name;
    this.lock = lock;
  }

  @Override
  public void run() {
    int distance = 100;
    synchronized (lock){
      try{
        System.out.println(name + " waiting...");
        lock.wait();
      }catch (InterruptedException e){
        e.printStackTrace();
      }
    }
    System.out.println(name + " started...");
    while (distance != 0){
      try{
        Thread.sleep((long) (100 * Math.random()));
      }catch (InterruptedException e){
        e.printStackTrace();
        break;
      }
      distance--;
      if (distance % 20 == 0 && distance != 0){
        System.out.println(name + " " + distance+ " miles left");
      }

      else if (distance == 0){
        System.out.println(name + " finished race!!!");
      }
    }
    System.out.println("_exiting from thread of " + name + " move simulation...");
  }
}

PS. Sorry for my bad english.

Thanks for you answers. So, is this solution more better?

public class TestThreads {
  public static void main(String[] args) throws InterruptedException {
    System.out.println("_start main");
    LightSignal lock = new LightSignal();
    Thread t1 = new Thread(new Car("Red car", lock));
    Thread t2 = new Thread(new Car("Black car", lock));
    t1.start();
    t2.start();
    synchronized (lock){
      Thread.sleep(1000L);
      lock.isGreen = true;
      System.out.println("Let`s go!");
      lock.notifyAll();
    }
    t1.join();
    t2.join();
    System.out.println("_exiting from main...");
  }
}

class Car implements Runnable {
  private final String name;
  private final LightSignal lock;

  public Car(String name, LightSignal lock) {
    this.name = name;
    this.lock = lock;
  }

  @Override
  public void run() {
    int distance = 100;
    synchronized (lock){
      try{
        while (!lock.isGreen){
          System.out.println(name + " waiting...");
          lock.wait();
        }
      }catch (InterruptedException e){
        e.printStackTrace();
      }
    }
    System.out.println(name + " started...");
    while (distance != 0){
      try{
        Thread.sleep((long) (100 * Math.random()));
      }catch (InterruptedException e){
        e.printStackTrace();
        break;
      }
      distance--;
      if (distance % 20 == 0 && distance != 0){
        System.out.println(name + " " + distance + " miles left");
      }
    }
    System.out.println(name + " finished race!!!");
    System.out.println("_exiting from thread of " + name + " move simulation...");
  }
}

class LightSignal {
  public boolean isGreen = false;
}

Your decision to use wait/notify is a bad one. You'll need much more programming around these raw primitives to get reliable behavior. For starters, you must *wait for a condition*, and not for the raw `notifyAll` signal. This is documented all around the web, for example [here](http://stackoverflow.com/questions/1038007/why-should-wait-always-be-called-inside-a-loop). — Marko Topolnik, May 06 '13 at 14:07

score 1 · Answer 1 · edited May 07 '13 at 08:15

When you call notifyAll() you need to change a state, and when you are waiting() you need to check that state in a loop. If you don't do this

notifyAll() only notifies thread which are waiting(), not thread which start waiting later.
wait() can wake spuriously.

In this case, the simplest solution is to not worry about it. You are waiting a random delay up to 100 ms, so trying to synchronize the start of such a random system is unlikley to make much difference.

I would think the "race is finished" when the loop exits, you don't need an if clause for this.

What is the good solution for calling notifyAll?

1 Answers1