Producer-Consumer using a stack in Java

Question

I am using a stack as my storage to implement my Producer-Consumer model.

public class Main {

/**
 * @param args
 */
public static void main(String[] args) {
    Stackable<Integer> stack = new MyArrayStack<Integer>();
    Producer producer = new Producer(stack);
    Consumer consumer = new Consumer(stack);
    producer.start();
    consumer.start();
}

private static class Producer extends Thread {
    public Producer(Stackable<Integer> s) {
        mStack = s;
    }

    private Stackable<Integer> mStack = null;
    private int mNumber = 0;

    @Override
    public void run() {
        // TODO generates number here
        while(true){        synchronized(this){
                while(!mStack.isEmpty())
                {
                    try{
                        this.wait();
                    } catch(Exception e)
                    {
                        e.printStackTrace();
                    }
                }
            mNumber++;
            System.out.println("Producer generates number:" + mNumber);
            mStack.push(mNumber);
            this.notifyAll();
            }
        }   
    }
}

private static class Consumer extends Thread {
    public Consumer(Stackable<Integer> s) {
        mStack = s;
    }

    private Stackable<Integer> mStack = null;

    @Override
    public void run() {
        // TODO consume number here.
        while(true){
            synchronized(this){
                while(mStack.isEmpty())
                {
                    try{
                        this.wait();
                    } catch(Exception e)
                    {
                        e.printStackTrace();
                    }
                }
            int number = mStack.pop();
            System.out.println("Consumer consumes number:" + number);
            this.notifyAll();
            }
        }}

}

}

Howver, when I test the program, it seems only the producer will work, it keeps generating numbers while consumer thread seems not be working.

Could expert help me debug where I went wrong in my code? Thanks.

Edit: My code for my stack is : public class MyArrayStack implements Stackable {

private static final int DEFAULT_SIZE = 16;
protected int sp; // empty stack
protected E[] head; // array
private int size;
private int count;

MyArrayStack(int size) {
    if (size <= 0)
          throw new IllegalArgumentException(
                     "Stack's capacity must be positive");
    head = (E[])new Object[size];
    sp = -1;
    count=0;

}






public boolean isFull() {
    return sp == this.size -1;
}

@Override
public void push(Object e) {

    if (!isFull())
    {
        head[++sp] = (E) e;
        count++;
    }

}

@Override
public E pick() {
    if (sp == -1)
        try {
            throw new Exception("Stack is empty");
        } catch (Exception e) {

            e.printStackTrace();
        }
    return head[sp];
}

@Override
public E pop() {
    count--;
    if (isEmpty()) {
        return null;
    } else
        return head[sp--];

}

@Override
public int count() {

    return count;
}

@Override
public boolean isEmpty() {

    return (sp == -1);
}
}

What happens in the consumer thread's run() method? Have you tried debugging this? — Dutts, May 09 '13 at 06:55
What do you see when you debug this? Do you see for example that the two threads are waiting and notifyAll() on unrelated objects? I suggest you implement a blocking take() and a blocking offer() which block when the stack is full or empty. Don't trying doing the externally or you are more likely to confuse yourself. — Peter Lawrey, May 09 '13 at 06:55
What does "yes" mean? When you ran your program in debug, the debugger screen was completely blank or it just threw lots of numbers at your which made no sense? The debugger doesn't "print nothing" — Peter Lawrey, May 09 '13 at 06:57
@PeterLawrey, It prints a lot of numbers which don't make any sense to me, they are in the format "Producer generates number:383349" and sth. like that. — Kevin, May 09 '13 at 07:00
are you getting any exception also. before this line System.out.println("Consumer consumes number:" + number); — Vineet Singla, May 09 '13 at 07:03
How is your pop() method implemented.. Will it give any exception at line nt number = mStack.pop();// Incase stack is empty. Post your code of pop() — Vineet Singla, May 09 '13 at 07:06
@Kevin, your code still wait/notifyAll on unrelated objects as I suggested so I assume that is the mostly likely cause of your problem. You really should learn how to use a debugger as this would solve your problem much faster. — Peter Lawrey, May 10 '13 at 06:29

score 1 · Answer 1 · answered May 09 '13 at 07:15

Oh . I have the issue, you are synchronising on this, instead of that synchronise on stack object, like synchronized(mStack){

then wait and notify on this object.

Adding code for you help. I had changed stack to list in my code , because i didnt had your stack object.

private static class Producer extends Thread {
    public Producer(List<Integer> s) {
        mStack = s;
    }

    private List<Integer> mStack = null;
    private int mNumber = 0;

    @Override
    public void run() {
        // TODO generates number here
        while(true){        synchronized(mStack){
                while(!mStack.isEmpty())
                {
                    try{
                        mStack.wait();
                    } catch(Exception e)
                    {
                        e.printStackTrace();
                    }
                }
            mNumber++;
            System.out.println("Producer generates number:" + mNumber);
            mStack.add(mNumber);
            mStack.notify();
            }
        }   
    }
}

private static class Consumer extends Thread {
    public Consumer(List<Integer> s) {
        mStack = s;
    }

    private List<Integer> mStack = null;

    @Override
    public void run() {
        // TODO consume number here.
        while(true){
            synchronized(mStack){
                while(mStack.isEmpty())
                {
                    try{
                         mStack.wait();
                    } catch(Exception e)
                    {
                        e.printStackTrace();
                    }
                }
            int number = mStack.remove(0);
            System.out.println("Consumer consumes number:" + number);
            mStack.notify();
            }
        }}

}}

Kuldeep Jain · Accepted Answer · 2013-05-09T07:53:22.477

To solve your problem you should synchronize and call wait(), notifyAll() on mStack and not this. By using this in your Producer you are waiting on producer object and in Consumer you are waiting on consumer object which is not correct. So you must call wait() as well as notifyAll() on same object which in your case should be mStack.

Here is the code snippet that works fine:

package com.thread.concurrency;

import java.util.LinkedList;
import java.util.List;

public class Main {

    public static void main(String[] args) {
        Main mainObj = new Main();
        List<Integer> stack = new LinkedList<Integer>();
        Producer producer = mainObj.new Producer(stack);
        Consumer consumer = mainObj.new Consumer(stack);
        producer.start();
        consumer.start();
    }

    private class Producer extends Thread {
        public Producer(List<Integer> s) {
            mStack = s;
        }

        private List<Integer> mStack = null;
        private int mNumber = 0;

        @Override
        public void run() {
            // TODO generates number here
            while (true) {
                synchronized (mStack) {
                    while(!mStack.isEmpty())
                    {
                        try{
                            mStack.wait(); // this.wait();
                        } catch(Exception e)
                        {
                            e.printStackTrace();
                        }
                    }
                mNumber++;
                System.out.println("Producer generates number:" + mNumber);
                mStack.add(mNumber);
                    mStack.notifyAll();// this.notifyAll();
                }
            }   
        }
    }

    private class Consumer extends Thread {
        public Consumer(List<Integer> s) {
            mStack = s;
        }

        private List<Integer> mStack = null;

        @Override
        public void run() {
            // TODO consume number here.
            while(true){
                synchronized (mStack) {
                    while(mStack.isEmpty())
                    {
                        try{
                            mStack.wait(); // this.wait();
                        } catch(Exception e)
                        {
                            e.printStackTrace();
                        }
                    }
                int number = ((LinkedList<Integer>) mStack).removeLast();
                System.out.println("Consumer consumes number:" + number);
                    mStack.notifyAll();// this.notifyAll();
                }
            }}

    }
}

Thanks, it starts processing again. But the result is still buch of "producer producing ....", I still couldn't see consumer thread. — Kevin, May 09 '13 at 07:32
@Kevin, Did you try my code snippet, it just works fine. Here is the output:Producer generates number:1 Consumer consumes number:1 Producer generates number:2 Consumer consumes number:2 Producer generates number:3 Consumer consumes number:3 ........ — Kuldeep Jain, May 09 '13 at 07:34
your code works perfectly. However, if I change the List back to my Stackable, then it is the same error. Maybe my stack implementation has issues in it? — Kevin, May 09 '13 at 07:41
yeah, that might be the case. But still you need to `synchronize` on `mStack` and not `this`. — Kuldeep Jain, May 09 '13 at 07:44
@KuldeepJain, One question, is it necessary to synchronize the shared resource in the producer? Please clear my confusion. — Suryaprakash Pisay, Mar 01 '18 at 13:40
@SuryaprakashPisay, yes it is. as `The current thread must own this object's monitor` for calling `wait()`. Please read about it at: https://docs.oracle.com/javase/7/docs/api/index.html?java/lang/Object.html — Kuldeep Jain, Mar 05 '18 at 20:04

Producer-Consumer using a stack in Java

2 Answers2