Producer Consumer Misunderstanding in Threading

Question

I want the ProducerThread to produce random values upto 10 and then expect ConsumerThread to consumer those values of Queue. Somewhere Producer is generating adding the values more than once. I have a concept that when we call notify on an object than that Thread would release lock and give chance to Thread which was expecting updation.

Here is the code, please correct my understanding.

public class ProducerThread extends Thread {

    Queue<Integer> values;

    ProducerThread(Queue<Integer> values) {
        this.values = values;
    }

    public void run() {
        while(true) {
            synchronized(values) {
                double totalValues = Math.random()*10;
                System.out.println("Going to populate total values:" + totalValues);

                for (int i = 1; i <= totalValues; i++) {
                    values.add(i);
                    System.out.println("Value updated: " + i);
                    try {
                        Thread.sleep(1000);
                    } catch (InterruptedException e) {
                        // TODO Auto-generated catch block
                        e.printStackTrace();
                    }
                }
                values.notify();
            }
        }
    }

}



public class ConsumerThread extends Thread {
    Queue<Integer> values;

    ConsumerThread(Queue<Integer> values) {
        this.values = values;
    }

    @Override
    public void run() {
        while(true) {
            synchronized (values) {

                try {
                    // Consumer Thread waits until values are populated by Producer Thread
                    if(values.isEmpty()) {
                        values.wait();
                    }

                    Iterator<Integer> iterateValues = values.iterator();
                    System.out.println("Going to consume values: " + values.size());
                    while (iterateValues.hasNext()) {
                        Integer removedValue = iterateValues.next();
                        System.out.println("Value deleted: " + removedValue);
                    }
                    values.clear();
                } catch (InterruptedException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
            }
        }
    }
}


public class Test {
    public static void main(String[] args) {
        Queue<Integer> values = new LinkedList<Integer>();
        ProducerThread producer = new ProducerThread(values);
        ConsumerThread consumer = new ConsumerThread(values);

        consumer.start();
        producer.start();

    }
}

I just ran this and it is doing what I read it should be doing. Can you please exactly clarify what you're expecting? Sample output, perhaps? — Joe C, Nov 20 '16 at 10:04
@JoeC I want the possible output as: the Producer set random values of 8 then it should populate queue from 1 to 8 values once Producer is done, Consumer will clear all 8 values. Then again Producer set random values to 5 and populate queue from 1 to 5 then consumer's turn should come. — Imam Bux, Nov 20 '16 at 10:33
You won't find any ready-made solution to making two (or more) threads take turns because that's not what threads are for. Anything that a group of threads can do by taking turns can be done more efficiently by a single thread. If it's a homework assignment, and you _must_ make threads take turns, then I would create one `java.util.concurrent.SynchronousQueue` for each thread. Each thread waits on its queue to receive a "token" (Object). When it gets the token, it does some work, and then puts the token into the next thread's queue and goes back to waiting again. — Solomon Slow, Nov 21 '16 at 03:28

Joe C · Accepted Answer · 2016-11-20T12:18:32.427

1

Aha! You have encountered the dreaded race condition!

Immediately after notify returns in your ProducerThread, said thread still has the lock. The ConsumerThread, woken up by the notify, will see that the lock is not available, and will wait until it becomes available.

Then the ProducerThread gives up the lock, it will then enter a race with the ConsumerThread to take that lock back (ProducerThread by way of re-entering the synchronized block, and ConsumerThread by means of having to return from wait). There is no guarantee which of these will win.

If you want your ProducerThread to wait for the items to be consumed before producing more, you should consider another wait/notify for that scenario.

EDIT: This image might help to explain things a bit more clearly.

edited Nov 20 '16 at 12:18

answered Nov 20 '16 at 10:43

Joe C

15,324
8
38
50

It works by using another wait and notify but why is it that ProducerThread enters into race to acquire lock again. It seems like on notify ProducerThread only looses lock but does not notify the thread which is waiting. I don't see wait/notify mechanish working here. Please correct me. – Imam Bux Nov 20 '16 at 11:56
1

Two things. Firstly, `notify` does not actually give up the lock. It only notifies another thread (`ConsumerThread` in this case) that whatever it was waiting for has now happened. Secondly, the `ConsumerThread` has to re-acquire the lock before returning from `wait`. If the `ProducerThread` gets the lock again first (by returning to the top of your loop and re-entering the `synchronized` block), then the `ConsumerThread` remains blocked. – Joe C Nov 20 '16 at 11:59
I've added a diagram to my answer, in case it helps to clarify things. – Joe C Nov 20 '16 at 12:18

Producer Consumer Misunderstanding in Threading

1 Answers1