Why is the order in which notify() is called in the following blocking queue example important?

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
    }
} 
Jon Skeet
people
quotationmark

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();
}

people

See more on this question at Stackoverflow