Single Producer and Consumer example in Thinking In Java 4

In TIJ4 P1208, there is one consumer (WaitPerson) and one producer (Chef). Each synchronize on itself when checking if there is meal available. I think they should synchronize on the meal instead. Otherwise, when the waitperson is checking if the meal is available, the chef could well be producing the meal, which makes the meal in an inconsistent state when waitperson is checking.

How do you guys think? Thanks

Here is the code:

import java.util.concurrent.;
import static net.mindview.util.Print.;


class Meal {
  private final int orderNum;
  public Meal(int orderNum) { this.orderNum = orderNum; }
  public String toString() { return "Meal " + orderNum; }
}

class WaitPerson implements Runnable {
  private Restaurant restaurant;
  public WaitPerson(Restaurant r) { restaurant = r; }
  public void run() {
    try {
      while(!Thread.interrupted()) {
        synchronized(this) {
          while(restaurant.meal == null)
            wait(); // ... for the chef to produce a meal
        }
        print("Waitperson got " + restaurant.meal);
        synchronized(restaurant.chef) {
          restaurant.meal = null;
          restaurant.chef.notifyAll(); // Ready for another
        }
      }
    } catch(InterruptedException e) {
      print("WaitPerson interrupted");
    }
  }
}

class Chef implements Runnable {
  private Restaurant restaurant;
  private int count = 0;
  public Chef(Restaurant r) { restaurant = r; }
  public void run() {
    try {
      while(!Thread.interrupted()) {
        synchronized(this) {
          while(restaurant.meal != null)
            wait(); // ... for the meal to be taken
        }
        if(++count == 10) {
          print("Out of food, closing");
          restaurant.exec.shutdownNow();
      开发者_运维技巧  }
        printnb("Order up! ");
        synchronized(restaurant.waitPerson) {
          restaurant.meal = new Meal(count);
          restaurant.waitPerson.notifyAll();
        }
        TimeUnit.MILLISECONDS.sleep(100);
      }
    } catch(InterruptedException e) {
      print("Chef interrupted");
    }
  }
}

public class Restaurant {
  Meal meal;
  ExecutorService exec = Executors.newCachedThreadPool();
  WaitPerson waitPerson = new WaitPerson(this);
  Chef chef = new Chef(this);
  public Restaurant() {
    exec.execute(chef);
    exec.execute(waitPerson);
  }
  public static void main(String[] args) {
    new Restaurant();
  }
} /* Output:
Order up! Waitperson got Meal 1
Order up! Waitperson got Meal 2
Order up! Waitperson got Meal 3
Order up! Waitperson got Meal 4
Order up! Waitperson got Meal 5
Order up! Waitperson got Meal 6
Order up! Waitperson got Meal 7
Order up! Waitperson got Meal 8
Order up! Waitperson got Meal 9
Out of food, closing
WaitPerson interrupted
Order up! Chef interrupted
*///:~

Otherwise, when the waitperson is checking if the meal is available, the chef could well be producing the meal

No, look at what the chef and waitperson are doing when synchronized on the same object:

synchronized on waitPerson:
   - waitperson is looking for meal [while (meal == null)]
   - chef is creating a meal [meal = new Meal()]
   - waitperson cannot look for meal exactly while chef is creating meal
   - waitperson waits/sleeps until meal is ready
   - chef notifies all when meal is created, which wakes waitperson


synchronized on chef:
   - waitperson is taking the meal [meal = null]
   - chef is waiting for meal to be taken [while (meal != null)]
   - chef cannot check if meal has been taken exactly while the waitperson is taking the meal
   - chef waits/sleeps until meal is taken
   - waitperson notifies all when meal is taken, which wakes chef

Because meal is either null or not null, only one of the synchronize blocks is "active" at a time. The code alternates between the blocks as meal becomes not null, then null, then not null, etc.

You often see code that synchronizes threads on a resource to protect the resource from simultaneous access. This common use case for synchronization may make the above code seem counter-intuitive - it synchronizes on the Runnable objects (chef and waitperson) rather that the meal "resource".

However, meals aren't ideal to use for synchronization in this case because they are so short-lived and the code isn't intended to protect the meal from simultaneous access by multiple threads. Rather, the code is simply trying to coordinate a producer and consumer thread. Using more stable, long-lived instance(s) to coordinate these threads is simpler than using shorter-lived meal instances.

It doesn't matter what they use to synchronize with as long as the instance(s) exist and the threads agree what instances to use to synchronize under what conditions. The code could have created two arbitrary token instances of two arbitrary classes to use for synchronization called MEAL_READY and MEAL_NOT_READY. However, the chef and waitperson instances makes convenient, ready-made, long-lived instances to use for synchronization.

The real problem here is that you need two conditions (1 wait for meal / produce meal and notify, another for wait for meal been eating / eat meal and notify). So you can use any two differents object like MEAL_READY and MEAL_NOT_READY like said by Bert F.

You can do that with conditions on a more clear way o even with a syncronous queue.