Threads producer consumer in java

Question

Below is the consumer producer problem code, but the code is not working as expected. Here the consumer and producer are supposed to be just producing and consuming one object.

public class ProducerConsumer {
    private static LinkedList<Integer> linkedList = new LinkedList<>();

    public static void main(String a[]) throws InterruptedException {
        Thread producer = new Thread(new Runnable() {

            @Override
            public void run() {
                synchronized(this) {
                    while (linkedList.size() == 1) {
                        try {
                            wait();
                        } catch(InterruptedException e) {
                            e.printStackTrace();
                        }
                    }
                    System.out.println("Produced");
                    linkedList.add(1);
                    notify();
                    try {
                        Thread.sleep(1000);
                    } catch(InterruptedException e) {
                        e.printStackTrace();
                    }
                }
            }
        });

        Thread consume = new Thread(new Runnable() {
            @Override
            public void run() {
                // produce
                synchronized(this) {
                    while (linkedList.isEmpty()) {
                        try {
                            wait();
                        } catch(InterruptedException e) {
                            e.printStackTrace();
                        }
                    }
                    System.out.println("Consumed");
                    linkedList.removeFirst();
                    notify();
                    try {
                        Thread.sleep(1000);
                    } catch(InterruptedException e) {
                        e.printStackTrace();
                    }
                }
            }
        });
        producer.start();
        consume.start();
        producer.join();
        consume.join();

    }
}

We get the output as : Produced

And the program hangs.

Please help with possible solutions/ explanations

Think of what happens if the producer thread runs first.. the condition in the while loop is not met and the thread ends. Same idea with the consumer thread — giorashc, Aug 23 '18 at 19:22
Threads are possibly executed in parallel in contrast to sequentially. Your systems scheduler is (more or less) allowed to interrupt a thread after any machine instruction and giving control to another thread. So you have absolutely no idea of the order how the statements of both threads are executed, could be any. — Zabuzard, Aug 23 '18 at 19:28
From this link -> https://www.geeksforgeeks.org/synchronized-in-java/ Only one thread can own a monitor at a given time. When a thread acquires a lock, it is said to have entered the monitor. All other threads attempting to enter the locked monitor will be suspended until the first thread exits the monitor. — Prasad, Aug 24 '18 at 05:23

Nathan Hughes · Answer 1 · 2018-09-05T14:24:25.713

Use a shared lock. In the posted code each Runnable is using itself as a lock so no actual locking takes place.

When a thread waits, another thread needs to call notify on the same lock in order to wake up the waiting thread. We know from your logging that the Producer thread does its thing, but since the notify acts on a lock that is not the same as the one the Consumer is using, the consumer thread never wakes up.

Changing the code to use a shared lock works:

import java.util.*;

public class ProducerConsumer { private static LinkedList linkedList = new LinkedList();

public static void main(String a[]) throws InterruptedException {
    final Object lock = new Object();
    Thread producer = new Thread(new Runnable() {
        @Override
        public void run() {
            synchronized (lock) {
                while (linkedList.size() ==1) {
                    try {
                        lock.wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
                System.out.println("Produced");
                linkedList.add(1);
                lock.notify();
                try {
                    Thread.sleep(1000);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
        }
    });

    Thread consume = new Thread(new Runnable() {
        @Override
        public void run() {
            // produce
            synchronized (lock) {
                while (linkedList.isEmpty()) {
                    try {
                        lock.wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
                System.out.println("Consumed");
                linkedList.removeFirst();
                lock.notify();
                try {
                    Thread.sleep(1000);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
        }
    });
    producer.start();
    consume.start();
    producer.join();
    consume.join();

}

}

Output for this is:

c:\example>java ProducerConsumer
Produced
Consumed

which I think is what you're expecting.

Btw see this other answer I wrote for a dirt-simple implementation of a queue; you are better off protecting the shared data structure than putting the code in the threads accessing the data structure, especially look at how much easier the code is to write.

Was about to write that. Please elaborate a bit more why he wasn't using the same lock before. — Johannes Kuhn, Aug 23 '18 at 19:34
It's even easier to use `ArrayBlockingQueue` instead of all that stuff with `LinkedList`, locks, `wait` and `notify` — Ivan, Aug 23 '18 at 19:43
@Ivan: definitely, I don't understand why this kind of code is so popular. It's much better to protect the shared data structure than to put locking in the individual threads, and the code for it is much simpler. Good reminder, I edited the answer to mention this. — Nathan Hughes, Aug 23 '18 at 19:52

Emmanuel H · Answer 2 · 2018-08-23T19:34:55.707

Concurrency means that you can not know before runtime which Thread will end first. So you can not know which of the Consumer and Producer is launched, executed or finished first.

To help you, you can use a cyclic barrier https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/CyclicBarrier.html or applying the Fork/Join Framework https://docs.oracle.com/javase/tutorial/essential/concurrency/forkjoin.html

Your synchronized blocs just say : only one Thread at a time can execute this part of code, not execute the first and the second after.

An example of how CyclicBarrier works :

service = Executors.newFixedThreadPool(numThreadsTotal);
CyclicBarrier c = new CyclicBarrier(numThreadsToWait);
runProducer();
c.await();
runConsumer();

It will wait until the there is as much Threads as numThreadsToWait that have execute the runProducer to execute the runConsumer().

Perhaps using a Thread Pool with a size of 1 could help you, but you will loose the benefits of concurrency.

score 0 · Answer 3 · answered Sep 05 '18 at 14:40

0

I think best what you can do, is use BlockingQueue.

answered Sep 05 '18 at 14:40

Grigoriev Nick

1,099
8
24

Threads producer consumer in java

3 Answers3

Linked