0

Is it wrong to use notifyAll() inside the loop in a multiple producer-consumer problem in Java?

Here is the code snippet which I am talking about.

public void run() {

        while(producer_counter <= 64) {

            synchronized(list) {

                threadId1 = "Producer " + Thread.currentThread().getName();

                //Buffer size is 8
                while(list.size() >= 8) {
                    System.out.println( "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
                    System.out.println(threadId1+ " found the buffer is full & waiting for a Consumer to consume.");
                    System.out.println( "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
                    try {

                        list.wait();
                        list.notifyAll();
                    }

                    catch (InterruptedException ie) {

                        ie.printStackTrace();
                    }
                }

                System.out.println(threadId1+ " produced item " + producer_counter);
                //Adding the items produced to the list.
                list.add(producer_counter);
                producer_counter++;
                //Invoking notify
                list.notifyAll();

            }
        }
    }
}
xingbin
  • 27,410
  • 9
  • 53
  • 103
Addy
  • 1
  • What's the point of notifying threads when you haven't changed anything? There's nothing they need to know about since nothing has changed since they decided to block. – David Schwartz Mar 14 '18 at 06:09

2 Answers2

1

notifyAll() is to notify all parties about changes in the state of the synchronized object. The first call, which is after wait(), is redundant because no state change happened. This call does not make the program wrong, but only causes waste of CPU cycles: when the buffer is full, notifyAll() is called and all other threads, waiting for the buffer's availability, go to processor only to call notifyAll() again and then call wait(). As a result, one processor in your machine will be always busy making unnecessary work.

BTW, you should not catch InterruptedException (or any other exception) if you don't know why it happened and what to do about it. If, like here, you cannot write public void run() throws InterruptedException, then write 

} catch (InterruptedException ie) {
    throw new RuntimeException(ie);
}

Instead of RuntimeException, better declare your own unchecked exception.

Alexei Kaigorodov
  • 13,189
  • 1
  • 21
  • 38
0

You should not use list.notifyAll() in while loop.After adding the items produced to the list list.add(producer_counter) you can use notify all. 

            // producer thread waits while list is full
            while (list.size() >= 8){
                list.wait();
            }

            // to insert the jobs in the list and plus 1 the counter
            list.add(producer_counter);
            producer_counter++;

            // notifies the consumer threads that now it can start 
           //consuming
            list.notifyAll();
maddy
  • 248
  • 1
  • 5
  • 16