Java Threads | Producer Consumer : What's wrong with the code?

Question

Trying to learn multithreading and inter-process communication of threads. Have implemented a typical producer-consumer problem. However, the output am getting is pretty sequential which should not be the case while working with threads ideally.

Ok, so here is the complete code:

public class ProducerConsumerSimulation {


public static ProducerConsumerSimulation simulation = new ProducerConsumerSimulation();


private Queue<Integer> sharedQueue = new LinkedList<Integer>();

private int MAX_LIMIT =  10;

public void produce(int i){
    synchronized (sharedQueue) {
        while(true){


            while(sharedQueue.size()==MAX_LIMIT){
                try {
                    sharedQueue.wait();
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            } 
            sharedQueue.add(i);
            System.out.println("Produced: "+i);
            sharedQueue.notifyAll();
            return;


        }
    }
}

public void consume() {
    synchronized (sharedQueue) {
        while (true) {


            while (sharedQueue.isEmpty()) {
                try {
                    sharedQueue.wait();
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            } 
            int i = sharedQueue.remove();
            System.out.println("Consumed: " + i);
            sharedQueue.notifyAll();
            return;


        }
    }
}

/**
 * @param args
 */
public static void main(String[] args) {


    for(int i=1;i<=5;i++){

        Runnable p = new Producer();
        Thread prod = new Thread(p);
        prod.start();
    }
    for(int i=1;i<=5;i++){

        Runnable c = new Consumer();
        Thread con = new Thread(c);
        con.start();
    }


}

}

Producer Thread :

public class Producer implements Runnable {

/* (non-Javadoc)
 * @see java.lang.Runnable#run()
 */
@Override
public void run() {

    for(int i=1;i<=10;i++){
        ProducerConsumerSimulation.simulation.produce(i);
    }

}

}

Consumer thread:

public class Consumer implements Runnable {

/* (non-Javadoc)
 * @see java.lang.Runnable#run()
 */
@Override
public void run() {

    for(int i=1;i<=10;i++){
        ProducerConsumerSimulation.simulation.consume();
    }

}

}

Now as I run the program, producer threads are always running first than consumer. Even if I create multiple producers/consumers, result is same. Below is the output on single producer and single consumer:

Produced: 1
Produced: 2
Produced: 3
Produced: 4
Produced: 5
Produced: 6
Produced: 7
Produced: 8
Produced: 9
Produced: 10
Consumed: 1
Consumed: 2
Consumed: 3
Consumed: 4
Consumed: 5
Consumed: 6
Consumed: 7
Consumed: 8
Consumed: 9
Consumed: 10

Can somebody explain the behavior here? I have gone through many answers here but want to understand if I have wrongly implemented or else.

Answer 1

First of all you should delete those two while (true) loops that actually only execute once since you return after the first iteration. It makes your code quite confusing.

Now, regarding your question: both the producer and the consumer don't do anything with between two productions/consumptions. They immediately re-enter the synchronized block to put/get something to/from the queue. A more realistic example would take some time to produce the values before putting them in the queue, and would take some time doing something with the value obtained from the queue. If you introduce an arbitrary delay after each call to produce() or consume(), you'll start seeing more interlaces productions and consumptions.

Finally, note that you're reinventing the wheel by using a LinkedList, synchronized blocks and calls to wait/notifyAll. You should just use a BlockingQueue, which would be all that for you.