NotifyAll, IllegalMonitorStateException

Question

import java.math.BigInteger;

class Numbers {

    final static int NUMBER = 2;
    final static int POWER = 4;

    static long msecs;
    static BigInteger result;
    static Boolean done = false;

    public static void main(String[] args) {

        BigInteger number = BigInteger.valueOf(NUMBER);
        result = number;

        //Boolean done = false;

        Runnable pow = () -> {
            System.out.println(number + " pow " + POWER + " = " + number.pow(POWER));

            synchronized (done) {
                done = true;
                done.notifyAll();
            }
        };

        Runnable sum = () -> {
            for(int i = 2; i<POWER; i=i*i) {
                result = result.multiply(result);
            }

            System.out.println(number + " sum " + POWER + " = " + result);

            synchronized (done) {
                done = true;
                done.notifyAll();
            }
        };

        Runnable time = () -> {
            for(msecs = 0; true; msecs++) {
                try {
                    Thread.sleep(1);
                } catch(InterruptedException e) {
                    //nic
                }
            }
        };

        Thread timet = new Thread(time);
        Thread sumt = new Thread(sum);
        Thread powt = new Thread(pow);

        timet.start();
        powt.start();

        synchronized (done) {
            while(!done) {
                try {
                    done.wait();
                } catch (InterruptedException e) {
                    //nic
                }
            }
        }

        timet.interrupt();
        powt.interrupt();

        System.out.println("Pow time " + msecs + " msecs.");
        done = false;

        timet.start();
        sumt.start();

        try {
            synchronized (done) {
                while (!done) {
                    done.wait();
                }
            }
        } catch (InterruptedException e) {
            //nic
        }


        timet.interrupt();
        sumt.interrupt();

        System.out.println("Sum time " + msecs + " msecs.");

    }
}

I want to check time differences between those two methods, but done.notifyAll() keeps throwing IllegalMonitorStateException.

Answer 1

I think I know what's happening.... If you read Java's documentation for Object.notifyAll()

IllegalArgumentException - if the value of timeout is negative. IllegalMonitorStateException - if the current thread is not the owner of the object's monitor. InterruptedException - if any thread interrupted the current thread before or while the current thread was waiting for a notification. The interrupted status of the current thread is cleared when this exception is thrown.

Therefore, I think you need to have a new Object that acts as a lock for accessing your done. For example: a private final Object doneLock = new Object(). Instead of synchronizing on Done, you would now synchronize on doneLock. I should be final because locks need to be private and final so they can't be accessed outside.

For example,

synchronize(doneLock) {
    done = true;
    doneLock.notifyAll();
}

Answer 2

Problem is here:

synchronized (done) {
    done = true;//<---problem
    done.notifyAll();
}

Since you are assigning new value to done it means you are executing notifyAll on Boolean.TRUE but your synchronization block is using monitor of Boolean.FALSE. And since notifyAll requires thread to posses monitor of object on which it is executed it throws IllegalMonitorStateException.

So don't change value of object on which you synchronize. Also avoid synchronizing on objects which are available for all classes (public constants /literals) since you are risking that other people will have same idea and will also use them in their synchronization which could cause you some pain.

Locks should be accessible only from within class to they belong. So take example from Jon Skeet (What is the difference between synchronized on lockObject and using this as the lock?) and synchronize on your own

private final Object lock = new Object();