Blocked using wait() and notify() in Java

Question

I am writing producer and consumer code using wait() and notify() in Java. Thread-0 is created and is invoked on produce() and Thread-1 is created and is invoked on consume().

public class Processor {

  private volatile List<Integer> list = new ArrayList<>();
  private final int MAX_CAPACITY = 5;
  Object lock = new Object();

  public void produce() throws InterruptedException {

    while (true) {

      while (list.size() == MAX_CAPACITY) {
        System.out.println("List is full! Producer is Waiting....");
        synchronized (lock) {
          lock.wait();
        }
      }

      synchronized (lock) {
        int random = new Random().nextInt(100);
        list.add(random);
        System.out.println("Added to list:" + random);
        lock.notify();
      }
    }
  }

  public void consume() throws InterruptedException {

    while (true) {

      while (list.size() == 0) {
        System.out.println("List is empty!! Consumer is Waiting...");
        synchronized (lock) {
          lock.wait();
        }
      }

      synchronized (lock) {
        int i = list.remove(0);
        System.out.println("Removed from list:" + i);
        lock.notify();
      }
    }
  }
}

The problem is that during execution, program stops after produce():

List is empty!! Consumer is Waiting...
Added to list:22
Added to list:45
Added to list:72
Added to list:91
Added to list:51
List is full! Producer is Waiting....

I am not able to understand what's the problem here. I somehow figured out that wrapping the code from while loop in synchronized block in produce() and consume() solves the problem.

produce()

synchronized (lock) {
                while (list.size() == MAX_CAPACITY) {
                    System.out.println("List is full! Producer is Waiting....");

                    lock.wait();
                }

consume

synchronized (lock) {
                while (list.size() == 0) {
                    System.out.println("List is empty!! Consumer is Waiting...");

                    lock.wait();
                }
            }

What is the issue here? Is it a case of Thread starvation or deadlock?

Edit: Calling class:

public class App {
    public static void main(String[] args) {
        final Processor processor = new Processor();

        Runnable r1 = new Runnable() {

            @Override
            public void run() {
                try {
                    processor.produce();
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }

            }
        };

        Runnable r2 = new Runnable() {

            @Override
            public void run() {
                try {
                    processor.consume();
                } catch (InterruptedException e) {

                    e.printStackTrace();
                }
            }
        };

        Thread t1 = new Thread(r1);
        Thread t2 = new Thread(r2);

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


    }
}

How do you call produce and consume methods? Could you please post that code? — Divers, Jul 21 '16 at 08:35
This is not a deadlock. A deadlock requires two locks or more. — user207421, Jul 24 '16 at 04:39

Peter Lawrey · Accepted Answer · 2016-07-21T08:39:53.557

6

When you perform list.size() it is not thread safe and there is no guarentee you will ever see the value changed in another thread. The JIT could even inline the value if it detects you are not changing it in that thread.

By placing the synchronized block outside the loop you ensure a change in the value is visible (as it is also inside the while(true) loop.

edited Jul 21 '16 at 08:39

answered Jul 21 '16 at 08:36

Peter Lawrey

525,659
79
751
1,130

I'd also add: "prefer using `notifyAll` over `notify`. – Mark Rotteveel Jul 21 '16 at 08:37
@MarkRotteveel - bad advice. NotifyAll is inefficient. If you need to rely on it, then something is suboptimal in your synchronization. – Stephen C Jul 21 '16 at 08:39
@StephenC Maybe, but if you have - for example - multiple producers and consumers, then consumers might wake up consumers and producers might wake up producers and if you are unlucky it might take a while before anything happens. – Mark Rotteveel Jul 21 '16 at 08:56
3

You should not have the producers and consumers waiting in the same thing. In fact, that is probably better implemented with a different synchronization construct; e.g. a Queue / Deque – Stephen C Jul 21 '16 at 13:21
@StephenC I agree, a BlockingQueue would be much better/simpler, I assume this is just an exercise and no one would actually do this. ;) – Peter Lawrey Jul 21 '16 at 13:38
1

@PeterLawrey I agree. But in most of the Java interview questions, these low level synchronization questions are rampant. – Anurag Jul 21 '16 at 15:15

score 1 · Answer 2 · edited May 23 '17 at 12:32

Using synchronized outside loop creates read barrier. Therefore producer/consumer will see latest list inside loop where you are checking list.size(). That is why it works after you move while loop inside synchronized block.

In your case I would also suggest you to use single synchronized block in producer/consumer.

For example in your implementation if list.size() == 0 becomes false for consumer, it will release lock on lock object then in the next statement try to re-acquire lock again for consuming the data, which is unnecessary and inefficient. It should be like:

public void consume() throws InterruptedException {

  while (true) {
    synchronized (lock) {
      while (list.size() == 0) {
        System.out.println("List is empty!! Consumer is Waiting...");

        lock.wait();
      }

      int i = list.remove(0);
      System.out.println("Removed from list:" + i);
      lock.notify();
    }
  }
}

Blocked using wait() and notify() in Java

2 Answers2