Search code examples
javamultithreadingsynchronizationsynchronizedproducer-consumer

wait() - notify() mechanism in java malfunctioning in a strange way


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!


Solution

  • 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");
        }
    }