Interthread communication Producer Consumer Problem

Question

I am trying to do some producer consumer poc with threads with below code and after one round both threads are in waiting state. but I am expecting them to keep on going in loop where one thread keep increasing counter and other decreasing.

can anyone please suggest what I am missing?

public class ProducerConsumerWithThreads {

        synchronized void withdrawBoxConsumer() {
            if(box > 0){
                box --;
                System.out.println("Took one box now boxes left "+  box);
            }
            if(box == 0) {
                System.out.println("Please put more boxes");
                notify();
                try{wait();}catch(Exception e){
                    System.out.println("Exception occured" + e.fillInStackTrace());
                }
            } else {
                withdrawBoxConsumer();
            }
        }

        synchronized void putBoxProducer() {
            if(box < 10){
                box ++;
                System.out.println("Put one box now boxes are "+  box);
            }
            if(box == 10) {
                System.out.println("Please Consume boxes");
                notify();
                try{wait();}catch(Exception e){
                    System.out.println("Exception occured" + e.fillInStackTrace());
                }
            } else {
                putBoxProducer();
            }
        }

        public int box = 5;

        public static void main(String[] args) throws InterruptedException {
            //pipeline of 10 boxes
            //consumer takes one at a time .. till its empty

            int boxLimit = 10;
            final int box = 5;
            final ProducerConsumerWithThreads c=new ProducerConsumerWithThreads();

            new Thread(){
                public void run(){c.withdrawBoxConsumer();}
            }.start();
            new Thread(){
                public void run(){c.putBoxProducer();}
            }.start();
        }
}

Output I am getting is:

Took one box now boxes left 4
Took one box now boxes left 3
Took one box now boxes left 2
Took one box now boxes left 1
Took one box now boxes left 0
Please put more boxes
Put one box now boxes are 1
Put one box now boxes are 2
Put one box now boxes are 3
Put one box now boxes are 4
Put one box now boxes are 5
Put one box now boxes are 6
Put one box now boxes are 7
Put one box now boxes are 8
Put one box now boxes are 9
Put one box now boxes are 10
Please Consume boxes

I am expecting it to continue further in loops as per logic! can anyone help?

matt · Accepted Answer

You're issue is a pretty basic flow issue: When 'notify' is called and the thread that was waiting starts again, the method completes and the thread stops running.

Note that both of your methods are synchronized on the same object, so they will never run at the same time. Eg. one thread will get the monitor, and increment/decrement the box until it finally waits. Then the other thread will go, until it waits.

You have a few other issues that are pretty common when using wait and notify.

    synchronized void withdrawBoxConsumer() {
        while( !Thread.currentThread().isInterrupted() ) { 
            if(box > 0){
                box --;
                System.out.println("Took one box now boxes left "+  box);
            }
            while(box == 0) {
                System.out.println("Please put more boxes");
                notifyAll();
                try{
                    wait();
                }catch(Exception e){
                    throw new RuntimeException(e);
                }
            } 
        }
    }

    synchronized void putBoxProducer() {
        while( !Thread.currentThread().isInterrupted() ) { 
            if(box < 10){
                box ++;
                System.out.println("Put one box now boxes are "+  box);
            }
            while(box == 10) {
                System.out.println("Please Consume boxes");
                notifyAll();
                try{
                    wait();
                }catch(Exception e){
                   throw new RuntimeException(e);
                }
            }
        }
    }

I made it non-recursive because the way you were doing it, the stack will overflow.
The wait condition is in a loop because of spurious wake-ups.
I switched to notifyAll notify would only wake one waiting thread, in this case it should be okay, but it is better to be safe.
box should ideally be a concurrent class or volatile, but since you're always working in a synchronized method it should be ok.
Likewise box++ and box-- are race conditions.
e.fillInStackTrace() is not what you want to use.

Interthread communication Producer Consumer Problem

Answers (2)

Related Questions