-1

I am refreshing my knowledge on Java and tried to write a Wait and Notify ( Producer, Consumer) type of multithreaded program. The idea is two have a producer thread that adds data to the array list and notifies the consumer thread which is waiting for on the lock attained by the producer thread. The code is giving output not the expected output.

enter image description here

But What I expected to see is

enter image description here

If you see Both the consumer threads are put on wait. When the 1st producer thread adds a data into the arraylist and it sends a notifyall signal to all waiting threads. I parked the Producer thread for 10s so that the waiting thread will have time to pick the array list and display the content of the array. At this point the size of the array should be 1.

package Threads_New;

import java.util.ArrayList;
import java.util.List;

public class Producer_Consumer2 {

    List<String> sharedList = new ArrayList();
    int count = 0;
    Object o = new Object();

    public void producer()

    {

        synchronized (o) {
            try {
                Thread.sleep(2000);
            } catch (InterruptedException ex) {
                ex.printStackTrace();
            }
            System.out.println(Thread.currentThread().getName()
                    + "-Adding a value-" + count);
            count++;
            sharedList.add("Apple-" + count);
            o.notifyAll();
            System.out.println("The Waiting thread is notified");
            try {
                Thread.sleep(10000);
            } catch (Exception ex) {
                ex.printStackTrace();
            }

        }

    }

    public void consumer() {

        System.out.println(Thread.currentThread().getName()
                + "- came inside consumer");
        synchronized (o) {
            try {
                o.wait();
                System.out.println(Thread.currentThread().getName()
                        + "- Notification recieved");
            } catch (InterruptedException ex) {
                ex.printStackTrace();
            }
            System.out.println("I was waiting and now got notified");
            for (String s : sharedList) {
                System.out.println("Data from the sharedList is:" + s);
            }

        }
    }

    public void ThreadStarter() {
        Thread[] T = new Thread[3];
        int run = 0;
        while (run < 2) {
            T[run] = new Thread(new Runnable() {

                @Override
                public void run() {
                    producer();
                }
            });

            T[run + 1] = new Thread(new Runnable() {

                @Override
                public void run() {
                    consumer();
                }
            });

            /*
             * for(int i=0;i<10;i++) { t1.start(); t2.start(); }
             */

            T[run].start();
            T[run + 1].start();
            run++;
        }

        /*
         * for(int i=0;i<21;i++) { try{ T[i].join(); T[i+1].join();
         * }catch(InterruptedException ex) { ex.printStackTrace(); } }
         */

    }

    public static void main(String[] args) {
        Producer_Consumer2 pc = new Producer_Consumer2();
        pc.ThreadStarter();
    }

}
sstan
  • 35,425
  • 6
  • 48
  • 66
GAK
  • 1,018
  • 1
  • 14
  • 33
  • If this is actually supposed to be used consider using a BlockingQueue instead https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/BlockingQueue.html – kervin Dec 09 '15 at 19:19
  • 1
    Trying to control multi-threaded code with sleeps never ends well. – sstan Dec 09 '15 at 19:22
  • Why is there a wait in your consumer at all? Are you sure you understand how these primitives work, along with synchronized? Generally, it's much easier to use the higher level library utilities in the concurrent package. – pvg Dec 09 '15 at 19:26
  • https://docs.oracle.com/javase/tutorial/essential/concurrency/guardmeth.html – Solomon Slow Dec 09 '15 at 19:35
  • @pvg, a consumer normally waits for something to consume. – Solomon Slow Dec 09 '15 at 19:56
  • @jameslarge that's silly. a consumer can wait without a literal call to 'wait'. synchronized will just as easily block a thread, for instance. – pvg Dec 09 '15 at 20:27

3 Answers3

1

Presumably, the sleep in your producer is to simulate a period of time in which it doesn't produce anything and the consumer is supposed to make forward progress during this time. But it can't because the producer code is still inside the synchronized block. You need to move the sleep to outside the synchronized block.

Also, your consumer calls wait without making sure the thing it's waiting for hasn't already happened. That's a very serious mistake because if you wait for something that's already happened, you may be waiting forever.

David Schwartz
  • 179,497
  • 17
  • 214
  • 278
0

I think at least part of the problem you are encountering is that the producer is holding the lock object even while it is sleeping. You should put the synchronized (o) block right around where you do notifyAll

ControlAltDel
  • 33,923
  • 10
  • 53
  • 80
0

wait() method should be called inside a loop to check for a satisfying condition so that predicate you're waiting on is true before continuing, otherwise it may result in spurious wakeups. Java doc link for wait() method:

http://docs.oracle.com/javase/6/docs/api/java/lang/Object.html#wait%28%29

akki
  • 423
  • 2
  • 12