Hello everyone, please help with the problem.

And so ... there are 3 manufacturers and 2 consumers, all different threads and all work simultaneously. There is a queue with 200 elements. Manufacturers add a random number from 1..100, and consumers take these numbers. If the queue of items> = 100 producers sleep, if there are no items in the queue - consumers sleep. If the elements become <= 80, producers wake up. All this works until the user has pressed the "q" button, after which the producers stop and consumers take all the elements, only then the program ends.

In my code, the last thread gets into wait() and the program hangs.

SimpleDataQueue.java

 public class SimpleDataQueue { private int head; private int tail; private volatile int elementsCount; private Integer[] myArrayQueue; public SimpleDataQueue(int size) { myArrayQueue = new Integer[size]; } public void add(Integer element) { synchronized (this) { while (elementsCount >= 100) { try { wait(); } catch (InterruptedException e) { e.printStackTrace(); } } myArrayQueue[head] = element; elementsCount++; if (head == myArrayQueue.length - 1) { head = 0; } else { head++; } notifyAll(); } } public Integer remove() { synchronized (this) { while (getElementsCount() == 0) { try { wait(); } catch (InterruptedException e) { e.printStackTrace(); } } Integer value = myArrayQueue[tail]; myArrayQueue[tail] = null; elementsCount--; if (tail == myArrayQueue.length - 1) { tail = 0; } else { tail++; } if (elementsCount <= 80) { notifyAll(); } return value; } } public synchronized int getElementsCount() { return elementsCount; } }

Producer.java

 import java.util.Random; public class Producer implements Runnable{ private SimpleDataQueue queue; private volatile boolean ready = false; public Producer(SimpleDataQueue queue) { this.queue = queue; } @Override public void run() { Random rand = new Random(); while(!ready) { try { Thread.sleep(rand.nextInt(1000)); } catch (InterruptedException e) { e.printStackTrace(); } queue.add(rand.nextInt(100)); System.out.println("Queue elements size is: " + queue.getElementsCount()); } System.out.println(" Ending " + Thread.currentThread().getName()); } public void shutdown() { ready = true; } }

Consumer.java

 import java.util.Random; public class Consumer implements Runnable { private volatile SimpleDataQueue queue; private volatile boolean ready = false; // private SomeUtil someUtil; public Consumer(SimpleDataQueue queue) { this.queue = queue; // someUtil = new SomeUtil(); } @Override public void run() { Random rand = new Random(); while(!(ready && (queue.getElementsCount() == 0))) { try { Thread.sleep(rand.nextInt(1000)); } catch (InterruptedException e) { e.printStackTrace(); } queue.remove(); System.out.println("Queue elements size is: " + queue.getElementsCount()); } System.out.println(" Ending " + Thread.currentThread().getName()); } public void shutdown() { ready = true; } }

Main.java

 import java.io.IOException; public class Main { public static void main(String[] args) throws InterruptedException, IOException { SimpleDataQueue queue = new SimpleDataQueue(200); Producer producer = new Producer(queue); Consumer consumer = new Consumer(queue); Thread t1 = new Thread(producer); Thread t2 = new Thread(producer); Thread t3 = new Thread(producer); Thread t4 = new Thread(consumer); Thread t5 = new Thread(consumer); t1.start(); t2.start(); t3.start(); t4.start(); t5.start(); Thread.sleep(5000); producer.shutdown(); t1.join(); t2.join(); t3.join(); consumer.shutdown(); t4.join(); t5.join(); System.out.println("Finish!"); } }

Console.log

 Queue elements size is: 1 Queue elements size is: 0 Queue elements size is: 0 Queue elements size is: 0 Queue elements size is: 0 Queue elements size is: 0 Queue elements size is: 1 Queue elements size is: 2 Queue elements size is: 3 Queue elements size is: 4 Queue elements size is: 5 Queue elements size is: 4 Queue elements size is: 3 Queue elements size is: 4 Queue elements size is: 3 Queue elements size is: 2 Queue elements size is: 3 Queue elements size is: 4 Queue elements size is: 5 Queue elements size is: 6 Queue elements size is: 7 Queue elements size is: 6 Queue elements size is: 5 Queue elements size is: 4 Queue elements size is: 5 Queue elements size is: 4 Queue elements size is: 5 Queue elements size is: 6 Queue elements size is: 7 Queue elements size is: 8 Queue elements size is: 9 Queue elements size is: 8 Queue elements size is: 9 Queue elements size is: 10 Queue elements size is: 9 Queue elements size is: 10 Queue elements size is: 9 Queue elements size is: 8 Queue elements size is: 9 Queue elements size is: 8 Queue elements size is: 9 Queue elements size is: 8 Queue elements size is: 7 Queue elements size is: 8 Queue elements size is: 9 Queue elements size is: 10 Queue elements size is: 11 Queue elements size is: 12 Queue elements size is: 11 Queue elements size is: 12 Queue elements size is: 13 Queue elements size is: 14 Ending Thread-2 Queue elements size is: 13 Queue elements size is: 12 Queue elements size is: 13 Ending Thread-1 Queue elements size is: 12 Queue elements size is: 13 Ending Thread-0 Queue elements size is: 12 Queue elements size is: 11 Queue elements size is: 10 Queue elements size is: 9 Queue elements size is: 8 Queue elements size is: 7 Queue elements size is: 6 Queue elements size is: 5 Queue elements size is: 4 Queue elements size is: 3 Queue elements size is: 2 Queue elements size is: 1 Queue elements size is: 0 Ending Thread-3

    2 answers 2

    @seroj , nothing surprising. You can and all Consumer-s hang.

    See what happens. Consumer checked ready and went to sleep (). At this point, you reset ready, but the sleeper after sleep () does not check for this, and goes to remove () where it hangs forever in wait (), since everyone who could have called notifyAll () has already completed.

    Only a trivial ready check after sleep () will not save you (the races will remain, only to hang less often ... The result is clear).

    You need an additional queue operation - stopQueue (). Get another ready in the SimpleDataQueue class and drop it when you call stopQueue () (also synchronized and calling notifyAll ()).

    Naturally, in while-in add and remove, this ready should be checked. Here, of course, there is an eternal Javish trouble with a single return value (after returning from add-remove, you should be able to reliably determine if the operation was successful).

    A possible option (not at all in the spirit of Java) is to return a sign via the additional argument add / remove (an array of one element).

    By the way, if you're interested, then in the “studies” there is such a topic (in C and C #), you can read.

    • yeah, here it is: hashcode.ru/research/221526 - VladD
    • Thank you all. It seems everything turned out. I don’t know how right everything is from the point of view of OOP and multithreading, but I hope that my code will be approved during the Junior Interval. In Consumer.java, added one if (after sleep () and before queue.remove ()); if (queue.getElementsCount ()> 0) {queue.remove (); } - seroj
    • @seroj, still note that it’s more appropriate to specifically close the queue. Have the on / off flag in it and act on it in the queue.add () and queue.remove () methods (without forgetting about notifyAll). At the very least, keep that in mind in your interview. - avp pm

    Option number 2, the problem may be that consumer flows will wait for the goods when there are no manufacturers, as an option to add a boolean variable to the queue class - are there any active manufacturers, and if they are not there, wait while trying to remove from the queue and return immediately .

    • Interesting ... While I was writing my answer, everything here (along with comments) has changed. - avp
    • I don’t know how bad it is, but when I realized that the answer was wrong, I deleted it along with the whole branch of comments, and then wrote another one) - zuev93