notifyAll() does not notify all threads

Question

This is my code and output are different anytime that I run the code. Sometimes all three readers will be notified and output is:

Waiting for calculation...

Waiting for calculation...

Waiting for calculation...

Finished

Total is: 4950Thread-1

Total is: 4950Thread-2

Total is: 4950Thread-0

and sometimes just two or one reader will be notified. what is the problem?

class Reader extends Thread {
    Calculator c;

    public Reader(Calculator calc) {
        c = calc;
    }

    public void run() {
        synchronized (c) {
            try {
                System.out.println("Waiting for calculation...");
                c.wait();
            } catch (InterruptedException e) {
            }
            System.out.println("Total is: " + c.total +Thread.currentThread().getName());
        }
    }

    public static void main(String[] args) {
        Calculator calculator = new Calculator();
        new Reader(calculator).start();
        new Reader(calculator).start();
        new Reader(calculator).start();
        new Thread(calculator).start();
    }
}

class Calculator implements Runnable {
    int total;

    public void run() {
        synchronized (this) {
            for (int i = 0; i < 100; i++) {
                total += i;
            }
            System.out.println("Finished");
            notifyAll();
        }
    }
}

As per the meta post, this question is claimed to be a duplicate, but both "duplicates" that have been dupe-hammered simply do not apply. How to use wait and notify in Java? reminds users that if you truly want to wait on the same object, you have to synchronize on that object. But this solution is already doing this. Java: notify() vs. notifyAll() all over again reminds users the difference between notify and notifyAll which is even further from the problem.

Answer 1

I was able to reproduce the problem - basically, if you have too fast of a computer, or a bad scheduling, it's possible for the calculator to finish before the reader has an opportunity to synchronize and wait.

c:\files\j>java Reader
Waiting for calculation...
Waiting for calculation...
Finished
Waiting for calculation...
Total is: 4950Thread-2
Total is: 4950Thread-0

To prevent this, you should verify that all your readers are ready before performing the calculation.

c:\files\j>java Reader Waiting for calculation... Waiting for readers... currently 1 Waiting for calculation... Waiting for calculation... Finished Total is: 4950Thread-1 Total is: 4950Thread-2 Total is: 4950Thread-0

Here is my code

import java.util.concurrent.atomic.AtomicInteger; // corsiKa added import
class Reader extends Thread {

    static class Calculator implements Runnable {
        int total;
        AtomicInteger readers = new AtomicInteger(0); // corsiKa added atomicinteger

        public void run() {
            // corsiKa added while
            while(readers.get() < 3) {
                System.out.println("Waiting for readers... currently " + readers.get());
                try { Thread.sleep(100); } catch(InterruptedException e) { }
            }
            synchronized (this) {
                for (int i = 0; i < 100; i++) {
                    total += i;
                }
                System.out.println("Finished");
                notifyAll();
            }
        }
    }

    Calculator c;

    public Reader(Calculator calc) {
        c = calc;
    }

    public void run() {
        synchronized (c) {
            try {
                c.readers.incrementAndGet(); // corsiKa added increment
                System.out.println("Waiting for calculation...");
                c.wait();
            } catch (InterruptedException e) {
            }
            System.out.println("Total is: " + c.total +Thread.currentThread().getName());
        }
    }

    public static void main(String[] args) {
        Calculator calculator = new Calculator();
        new Reader(calculator).start();
        new Reader(calculator).start();
        new Reader(calculator).start();
        new Thread(calculator).start();
    }
}

Answer 2

The problem with your code is that the thread's start order is not defined. The intuitively expected behavior is that the readers start first, then the calculator. This is exactly what happened in the log you show above.

But there are other interleavings possible as well. Calling Thread.start does not make any guarantees about the order of the starting. Assume two readers start first, then the calculator, then the last reader. In this case the calculator can enter the critical section before the third reader. The call of notifyAll from the calculator then happens before the third reader executes its wait call. Hence the third reader is damned to wait forever as not other call of notify or notifyAll will occur on the lock object.

One possible solution to your problem is to use a CountDownLatch which allows you to let the calculator wait until all three readers are ready.

Answer 3

Make your main method check the other Reader's states.

public static void main(String[] args) {
    Calculator calculator = new Calculator();
    Reader read1 = new Reader(calculator);
    read1.start();
    Reader read2 = new Reader(calculator);
    read2.start();
    Reader read3 = new Reader(calculator);
    read3.start();

    while(read1.getState() != Thread.State.WAITING && 
          read2.getState() != Thread.State.WAITING &&
          read3.getState() != Thread.State.WAITING){
        try {
            Thread.sleep(100);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }

    new Thread(calculator).start();
}

This will insure that you absolutely have all the threads waiting, and yields the same result every time.