wait() - notify() mechanism in java malfunctioning

Question

This question already has an answer here:

• Why should wait() always be called inside a loop 8 answers

I've tried reading some answers to similar questions here (I always do that) but did not find (or did not understand?) the answer to this particular issue.

I am implementing a fairly simple consumer-producer class, which receives elements to a list from a different thread and consumes them repeatedly. The class has the following code:

public class ProduceConsume implements Runnable
{

    LinkedList<Integer> _list = new LinkedList<Integer>();

    public synchronized void produce(Integer i)
    {
        _list.add(i);
        notify();
    }

    public void run()
    {
        while(true)
        {
            Integer i = consume();

            // Do something with the integer...
        }
    }

    private synchronized Integer consume()
    {
        if(_list.size() == 0)
        {
            try
            {
                wait();
            }
            catch(InterruptedException e){}

            return _list.poll();
        }
    }
}

The problem is - it usually works fine, but sometimes, the execution gets to

return _list.poll();

with the list still empty. I can't wrap my head around it - am I doing something terribly wrong? Shouldn't the runnable thread, which repeatedly tries to poll detect a zero length list, wait, and be awakened only after the producer method is done, hence making the list non-empty?

Nothing else "touches" the class from the outside, except for calls to produce. No other threads are synchronized on the runnable class.

By the way, for several reasons, I wish to use my own variant and not classes such as CopyOnWriteArrayList, etc.

Thanks! Any help would be greatly appreciated.

P.S - I have not used the wait-notify many times, but when I did, in the past, it worked. So if I apologize if I made some huge stupid error!

Answer 1

Since wait releases the lock you can't reason based on conditions tested before it started waiting, assuming the condition must have changed once wait is exited is not valid. You need to call wait in a loop, so that once the thread ceases waiting and takes the lock again, it checks that the condition it's waiting for has the expected value:

private synchronized Integer consume()
{
    try {
        while (_list.size() == 0) {
            wait();
        }            
    } catch (InterruptedException e) {
        Thread.currentThread().interrupt();
    }
    return _list.poll();
}

From the Oracle tutorial:

Note: Always invoke wait inside a loop that tests for the condition being waited for.

Also it's not safe to assume that just because wait returned that something sent a notification. wait can return even if there is no notification (the spurious wakeup).

It's hard to say what caused what you're seeing without a complete working example.

The linked Oracle tutorial page has a Producer Consumer example you might want to look at.

Answer 2

As the Javadoc for Object.wait states

As in the one argument version, interrupts and spurious wakeups are possible, and this method should always be used in a loop:

 synchronized (obj) {
     while (<condition does not hold>)
         obj.wait();
     ... // Perform action appropriate to condition
 }

Additionally, you shouldn't ignore an exception like InterruptedException. This will look like a spurious wake up and as you say produces an error.

private synchronized Integer consume() {
    try {
        while (_list.isEmpty()) 
            wait();
        return _list.poll();
    } catch(InterruptedException e) {
        throw new IllegalStateException("Interrupted");
    }
}