IllegalMonitorStateException calling wait() and notify()

Question

I've read the example of producer and consumer, and I changed a little. Pro() will print "first", and con() will print "second". I want every "second" appears after "first".

public class test {
    test() { }

    synchronized void pro() {
        new Thread(new Runnable() {
            @Override
            public void run() {
                while (true) {
                    System.out.println("First!");
                    notify();
                    try {
                        wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
            }       
        }).start();
    }

    synchronized void con() {
        new Thread(new Runnable() {
            @Override
            public void run() {
                while (true) {
                    try {
                        wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    System.out.println("Second!");              
                    notify();                   
                }
            }
        }).start();
    }

    public static void main(String[] args) {
        test m = new test();
        m.pro();
        m.con();
    }
}

The errors that occur are:

First!
Exception in thread "Thread-0" Exception in thread "Thread-1"
java.lang.IllegalMonitorStateException
    at java.lang.Object.notify(Native Method)
    at test$1.run(test.java:12)
    at java.lang.Thread.run(Unknown Source)
java.lang.IllegalMonitorStateException
    at java.lang.Object.wait(Native Method)
    at java.lang.Object.wait(Object.java:503)
    at test$2.run(test.java:32)
    at java.lang.Thread.run(Unknown Source)

Answer 1

There are a couple problems occurring simultaneously here.

The first is the obvious IllegalMonitorStateException. This exception occurs when wait or notify is called on an object when the calling thread doesn't hold that object's monitor lock. Even though the calls syntactically occur within a synchronized method, the actual calls to wait and notify occur on different threads and operate on different objects.

What's happening is that your pro and con methods are synchronized on your instance. Since they're called from the main method, the main thread is the one that takes the monitor locks. But these methods simply start other threads and then release the locks.

The wait and notify calls are implicitly this.wait and this.notify and they are within Runnable anonymous inner classes, so in these cases the this refers to the Runnable instances, not the outer test instance that's been synchronized. The rule is that the same thread that calls wait (or notify) on an object must also hold the monitor lock on the object. Since that's not the case, IllegalMonitorStateException is thrown. An additional problem is that the wait and notify calls are on different Runnable instances. Different threads must use wait/notify on the same object in order to communicate.

What you need to do is rearrange the code so that the synchronized blocks occur in the threads that do the actual synchronization. Then, you want to make sure that the synchronized, wait, and notify operations are all on the same object. Perhaps the best way to do that is to store your test instance in a static field (say, make m be a static field instead of a local variable). The run methods are members of the Runnable instances, so you can't just make these methods synchronized. Instead, you have to use synchronized blocks and pass the instance on which you want to synchronize. You can also remove the synchronized keywords from the pro and con methods since these methods don't actually need synchronization. The resulting code will look something like this:

static test m = new test();

void pro() {
    new Thread(new Runnable() {
        @Override
        public void run() {
            synchronized (m) {
                ...
                m.notify();
                ...
                m.wait();
                ...
            }
        }
    }).start();
}

You'll have to do something similar to the con method.

Now, the code should run without throwing IllegalMonitorStateException. But this uncovers a second problem: it might not work. It might deadlock because of the lost wakeup problem. A full explanation is beyond the scope of this answer. Briefly though, if the thread started by pro runs first, it might call notify when no thread is waiting. If this occurs, nothing happens. Then that thread continues and calls wait. Next, the con thread might start, and the first thing it does is call wait. Now both threads are blocked in wait and a notify will never occur.

The way to solve this is to have waiting and notification based on the state of the object. It won't work to call wait without checking the state of the object, nor will it work to call notify without changing the object's state.

For a full explanation of how to use wait and notify properly, see Goetz, Java Concurrency in Practice, chapter 14.

Answer 2

In order to be able (without IllegalMonitorStateException) to call wait() and notify() methods of some object, thread has to acquire object monitor first. That means this methods should be called either in synchronized block by the same object you wait() for, or in synchronized method, where you will be able to call this.wait().

Now let's take a look at your code:

@Override
public void run() {
    while (true) {
        try {
            wait();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println("Second!");              
        notify();                   
    }
}

You wait() and notify() for anonymous Runnable instance. This is probably a bug caused by misunderstanding of wait/notify mechanics. Try reading this article for further information.