Producer-consumer problem - both end up waiting

Question

new to multithreading. I wrote this program which should be a solution to the producer-consumer problem. The problem is that both a producer and a consumer end up in the waiting state. What seems to be wrong? (And everything else what is wrong ^_^) Thanks in advance.

Main class:

package producer.consumer2;

import java.util.Scanner;

public class Main {

    public static void main(String[] args) {
        Buffer<Integer> bf = new Buffer<>(10);
        Producer prod = new Producer(bf);
        Consumer cons = new Consumer(bf);
        prod.setConsumer(cons);
        cons.setProducer(prod);


        new Thread(prod).start();
        new Thread(cons).start();

        if(quitInput()) {
            prod.terminate();
            cons.terminate();
        }


    }

    private static boolean quitInput() {
        Scanner sc = new Scanner(System.in);
        String line = sc.nextLine();
        do {
            if(line.toLowerCase().equals("q") || line.toLowerCase().equals("quit")) {
                sc.close();
                return true;
            }
            line = sc.nextLine();
        } while(true);
    }

}

Buffer class:

package producer.consumer2;


import java.util.ArrayList;
public class Buffer<E> {
    private final int MAX_LENGTH;
    private ArrayList<E> values;

    public Buffer(int length){
        MAX_LENGTH = length;
        values = new ArrayList<E>(length);
    }

    public synchronized void add(E e) {
        if(values.size() < MAX_LENGTH) {
            values.add(e);
            System.out.println(values);
        } else {
            throw new RuntimeException("Buffer is full at the moment.");
        }
    }

    public synchronized boolean isEmpty() {
        return values.size() == 0;
    }

    public synchronized boolean isFull() {
        return values.size() >= MAX_LENGTH ? true : false;
    }

    public synchronized E remove(int index) {
        E val = values.remove(index);
        System.out.println(values);
        return val;
    }
}

Consumer class:

package producer.consumer2;

public class Consumer implements Runnable {

    private final Buffer<Integer> bf;
    private volatile boolean running = true;
    private Producer prod;

    public Consumer(Buffer<Integer> bf) {
        this.bf = bf;
    }

    public void setProducer(Producer prod) {
        this.prod = prod;
    }

    @Override
    public void run() {
        int sum = 0;
        int counter = 0;
        while (running) {
            if (bf.isEmpty()) {
                if (prod != null) {
                    synchronized (prod) {
                        prod.notify();
                    }
                }
                myWait(0);
            } else {
                sum += bf.remove(0);
                counter++;
            }
        }
        System.out.println("for first " + counter + " nums an avg = " + ((double) sum / counter));
    }

    private void myWait(long millisecs) {
        System.out.println("consumer is waiting.");
        try {
            synchronized (this) {
                this.wait(millisecs);
            }
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println("consumer is NOT waiting.");
    }

    public void terminate() {
        this.running = false;
    }

}

Producer class:

package producer.consumer2;

public class Producer implements Runnable {

    private final Buffer<Integer> bf;
    private volatile boolean running = true;
    private Consumer cons;

    public Producer(Buffer<Integer> bf) {
        this.bf = bf;
    }

    public void setConsumer(Consumer cons) {
        this.cons = cons;
    }

    @Override
    public void run() {
        int counter = 1;
        while (running) {
            if (bf.isFull()) {
                if (cons != null) {
                    synchronized (cons) {
                        cons.notify();
                    }
                }
                myWait(0);
            } else {
                bf.add(counter);
                counter++;
            }
        }
    }

    private void myWait(long millisecs) {
        System.out.println("producer is waiting.");
        try {
            synchronized (this) {
                this.wait(millisecs);
            }
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println("producer is NOT waiting.");
    }

    public void terminate() {
        this.running = false;
    }

}

Answer 1

Looks like a regular case of 'missed signal'. Since both consumer and producer just wait without checking a condition, yu have no way to ensure the notify actually happens during the waiting.

e.g. in Consumer :

if (prod != null) {
    synchronized (prod) {
        prod.notify();
    }
}
myWait(0);

Note that if, after prod.notify() the Production thread does all of its work, and notifies the consumer, before it even starts waiting, the consumer will start waiting for a signal that's already been given, and missed. Always take into account that waiting may not be needed anymore. So always check a condition before even starting to wait. In your case here, the consumer should not even begin waiting if the buffer is full. And likewise the producer should not start waiting if the buffer is empty. It's also possible to get spurious wake ups. So you'll have to re-check the condition when returning from waiting. The typical idiom is this :

synchronized(monitor) {
    while (!stateBasedCondition) {
        monitor.wait();
    }
}