Multiple read writes on a single static integer index causes index to go out of bounds

Question

I was trying to learn multi threading, and was trying out a simple producer/consumer pattern with wait and notify. When i split the pattern with two consume and one produce i get an ArrayIndexOutOfBounds Exception which is not clear. The issue does not occur always it occurs at times. I am using a I3 processor.

I have tried added an if block to check if the count variable is going below or above the declared size and still the issue persists.

    private static Object key = new Object();
    private static int[] buffer;
    private volatile static Integer count;

    static class Consumer {

        void consume() {
            synchronized (key) {
                if (isEmpty()) {
                    try {
                        key.wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
                buffer[--count] = 0;
                key.notify();
            }
        }

        public boolean isEmpty() {
            return count == 0;
        }
    }

    static class Producer {

        void produce() {
            synchronized (key) {
                if (isFull()) {
                    try {
                        key.wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
                buffer[count++] = 1;
                key.notify();
            }
        }

        public boolean isFull() {
            return count == buffer.length;

        }
    }
    public static void main(String[] args) throws InterruptedException {
        buffer = new int[10];
        count = 0;
        Producer producer = new Producer();
        Consumer consumer = new Consumer();
        Runnable produce = () -> {
            for (int i = 0; i < 1500; i++)
                producer.produce();
            System.out.println("Done producing");
        };
        Runnable consume = () -> {
            for (int i = 0; i < 1300; i++)
                consumer.consume();
            System.out.println("Done consuming");
        };
        Thread producerWorker = new Thread(produce);
        Thread consumerWorker = new Thread(consume);
        producerWorker.start();
        consumerWorker.start();
        //consumerWorker.join();
        Runnable checker = () -> {
            System.out.println("Lanched Delayed Consumer");
            for (int i = 0; i < 200; i++)
                consumer.consume();
        };
        Thread delayedConsumer = new Thread(checker);
        delayedConsumer.start();
        producerWorker.join();
        System.out.println("Data in Buffer " + count);
    }

 }

Expected it to be :

Lanched Delayed Consumer
Done consuming
Done producing
Data in Buffer 0

but got :

Lanched Delayed Consumer
Exception in thread "Thread-1" Exception in thread "Thread-0" java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 10
    at com.multi.thread.waitandnotify.WaitNotifyRunner$Consumer.consume(WaitNotifyRunner.java:27)
    at com.multi.thread.waitandnotify.WaitNotifyRunner.lambda$1(WaitNotifyRunner.java:78)
    at java.base/java.lang.Thread.run(Thread.java:835)
java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 10
    at com.multi.thread.waitandnotify.WaitNotifyRunner$Producer.produce(WaitNotifyRunner.java:55)
    at com.multi.thread.waitandnotify.WaitNotifyRunner.lambda$0(WaitNotifyRunner.java:73)
    at java.base/java.lang.Thread.run(Thread.java:835)
Data in Buffer 0

Answer 1

The problem is that:

• you have only one wait condition that you share for both the "full" and the "empty" state
• you're running two consumer threads ("consumerWorker" and "delayedConsumer") and one producer ("producerWorker")
• you're not re-checking the condition in a while-loop like you should

A possible thing that happens is:

1. Both consumer threads see that the buffer is empty
2. The producer thread puts one item in the buffer, and notifies.
3. A single consumer thread wakes up and processes one item. The buffer is now empty. It then notifies.
4. The second consumer thread wakes up (instead of the producer like you intended), doesn't check whether the buffer is empty, and tries to access the buffer at index -1.

You need to recheck the condition after you wake up from a wait call. Change the if to a while. Here's how to do that for the consumer; you need to do the same for the producer.

    void consume() {
        synchronized (key) {
            while (isEmpty()) {
                try {
                    key.wait();
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
            buffer[--count] = 0;
            key.notify();
        }
    }

That way the consumer won't get confused by the notify from the other consumer thread.

This is also "recommended" by the documentation of Object.wait:

The recommended approach to waiting is to check the condition being awaited in a while loop around the call to wait, as shown in the example below. Among other things, this approach avoids problems that can be caused by spurious wakeups.

(I quoted "recommended" because it's required to do it this way in order to implement it correctly because wait can also spuriously wake up without a call to notify)