I wrote a simple Blocking Queue example for a Prod-Cons problem. The example below won't work; unless I swap the add/remove part of the enqueue/dequeue logic with notify on the waiting thread. I couldn't find any clear explanation of this behavior in any implementation of BlockingQueue. In the enqueue part, shouldn't it be correct to add the element and then notify? In this way I can guarantee that when Consumer thread runs, it shall have a element to consumer and won't return null. Please explain.
import java.util.LinkedList;
import java.util.Queue;
public class BlockingQueueCustom<T> {
private int size = 10;
private Queue<T> queue = new LinkedList<T>();
public BlockingQueueCustom(int s) {
this.size = s;
}
public synchronized void enqueue(T element) throws InterruptedException {
while (queue.size() == size) {
wait();
}
queue.add(element); //Comment this part to make it work
if (queue.size() == 0) {
this.notifyAll();
}
//queue.add(element); Uncommenting this will make it work
}
public synchronized T dequeue() throws InterruptedException {
while (queue.size() == 0) {
this.wait();
}
T element = queue.remove(); //Comment this part to make it work
if (queue.size() == size) {
this.notifyAll();
}
return element; //Comment this part to make it work
//return queue.remove(); Uncommenting this will make it work
}
}
In the enqueue part, shouldn't it be correct to add the element and then notify?
That part doesn't really matter, as you're in a synchronized method - the other threads won't get to run until you've left the enqueue
method anyway. However, it does matter in terms of the test for size()
. Look here:
if (queue.size() == 0)
How do you expect that to be true if you've just added an element? You could check for it being 1 instead, meaning that before you added the element it must have been 0. Or you could keep a separate variable:
boolean shouldNotify = queue.size() == 0;
queue.add(element);
if (shouldNotify) {
this.notifyAll();
}
See more on this question at Stackoverflow