java wait and notify

Question

I'm taking one Integer variable and sharing with two threads. One thread should print even numbers and one thread should print odd number sequentially. But notify() throwing IllegalMonitorStateException.

package mywaitnotifytest;
public class App {

    public static void main(String[] args) {
        Integer i=0;
        Even even = new Even(i);
        even.setName("EvenThread");
        Odd odd = new Odd(i);
        odd.setName("OddThread");
        even.start();
        odd.start();
    }
}

class Even extends Thread{

    Integer var;

    Even(Integer var){
        this.var=var;
    }

    @Override
    public void run() {
        while(true){
            synchronized (var) {
                if(var%2==0){
                    try {
                        var.wait();
                    } catch (InterruptedException e) {
                        // TODO Auto-generated catch block
                        e.printStackTrace();
                    }
                }
                var++;
                System.out.println(Thread.currentThread().getName()+"  "+var);
                var.notify();
            }
        }

    }
}

class Odd extends Thread{

    Integer var;

    Odd(Integer var){
        this.var=var;
    }

    @Override
    public void run() {
        while(true){
            synchronized (var) {
                if(var%2!=0){
                    try {
                        var.wait();
                    } catch (InterruptedException e) {
                        // TODO Auto-generated catch block
                        e.printStackTrace();
                    }
                }
                var++;
                System.out.println(Thread.currentThread().getName()+"  "+var);
                var.notify();
            }
        }
    }
}

And the output is :

OddThread 1

Exception in thread "OddThread" java.lang.IllegalMonitorStateException

at java.lang.Object.notify(Native Method)

at mywaitnotifytest.Odd.run(App.java:67)

This question seems different as OP is getting exception on `notify`, not `wait`. Plus the reason of the exception is quite different and as nothing to do with non `synchronized` code — ortis, Apr 12 '16 at 07:42
You are not calling `notify()` on the same object as you locked. In short, don't lock on a mutable field. When you mutate it you change it. Also don't lock on a pooled object, as `Integer` is as this will have confusing consequences. — Peter Lawrey, Apr 12 '16 at 07:45
@ortis while the OP is using `synchornized` he/she is not calling notify on an object they have `synchronized`. — Peter Lawrey, Apr 12 '16 at 07:46
@NareshMuthyala Using `synchronized` means you have to call `notify`/`wait` on the same object. It doesn't give you permission to lock on any object. — Peter Lawrey, Apr 12 '16 at 07:47
For the above requirement what is the edit suggested in the code? How do i share the same object for multiple threads? — Naresh Muthyala, Apr 12 '16 at 07:53

Peter Lawrey · Answer 1 · 2016-04-12T09:07:39.357

I think this is sufficiently different to the usual answer to give another one.

In this case you are using synchronized. When you apply a lock it is on a object not a reference.

synchronized (var) {

This locks the object var references, not on var as a field.

var++;

This replaces the object var points to. It is the same as

var = Integer.valueOf(var.intValue() + 1);

Note: Integer and indeed all the primitive wrappers are Immutable. When you perform any operation on them you are actually unboxing, calculating using the primitive value and re-boxing the object. It is possible to get the same object back if it is pooled. e.g.

Integer i = 10;
i += 0; // gives back the same object.

However, if the object is not pooled

Double d = 10;
d += 0; // creates a new object.

var.notify();

Attempts the call notify on the new object, not the one which was locked.

You shouldn't attempt to lock a field which you mutate. It won't do what it appears to do. You also shouldn't lock on a pooled object. In this case you could have another thread using the same Integer for an unrelated purpose and notify() will wake up an unrelated thread.

To use wait/notify correctly, you should

notify() or notifyAll() after a state change in another shared field.
you should use a while loop for wait() to check the state change.

If you don't do this

notify can be lost if another thread is not waiting.
wait can wake spuriously, even when no notify was called.

For the above requirement what is the edit suggested in the code? How do i share the same object for multiple threads?

public class PingPong implements Runnable {    
    static class Shared { int num; }

    private final Shared var;
    private final int bit;

    public static void main(String[] args) {
        Shared var = new Shared();
        new Thread(new PingPong(var, 0), "EvenThread").start();
        new Thread(new PingPong(var, 1), "OddThread").start();
    }

    PingPong(Shared var, int bit) {
        this.var = var;
        this.bit = bit;
    }

    @Override
    public void run() {
        try {
            String name = Thread.currentThread().getName();
            while (true) {
                synchronized (var) {
                    while (var.num % 2 == bit)
                        var.wait();

                    var.num++;
                    System.out.println(name + "  " + var.num);
                    var.notify();
                }
            }
        } catch (InterruptedException e) {
            System.out.println("Interrupted");
        }
    }
}

This is why you should always use `final` when dealing with `synchronized`, `wait`, or `notify` — ortis, Apr 12 '16 at 07:55
@ortis where ever possible, with serialize objects it's not always possible to use `final` but the field should be effectively final. +1 — Peter Lawrey, Apr 12 '16 at 07:56
Upvoted this answer. Explains how wait should be used (in a while(conditionNotMet) {waitOnTheLockObject;}). This answer should be accepted. @PeterLawrey, along with this, if you edit and explicitly mention that all wrapper classes for primitives are immutable, it will be nice. It will explain why var++ on Integer will create a new Integer object (explicitly). — Amudhan, Apr 12 '16 at 09:00

score -1 · Answer 2 · answered Apr 12 '16 at 08:17

Instead of using Integer wrapper class,I created my own class and now It works fine.

package mywaitnotifytest;

public class App {
    public static void main(String[] args) {
        MyInt i = new MyInt(0);
        Even even = new Even(i);
        even.setName("EvenThread");
        Odd odd = new Odd(i);
        odd.setName("OddThread");
        even.start();
        odd.start();
    }
}

class Even extends Thread {

    MyInt var;

    Even(MyInt var) {
        this.var = var;
    }

    @Override
    public void run() {
        while (true) {
            try {
                Thread.sleep(200);
            } catch (InterruptedException e1) {
                // TODO Auto-generated catch block
                e1.printStackTrace();
            }
            synchronized (var) {
                if (var.i % 2 == 0) {
                    try {
                        var.wait();
                    } catch (InterruptedException e) {
                        // TODO Auto-generated catch block
                        e.printStackTrace();
                    }
                }
                var.i++;
                System.out.println(Thread.currentThread().getName() + "  " + var.i);
                var.notify();
            }
        }

    }

}

class Odd extends Thread {
    MyInt var;

    Odd(MyInt var) {
        this.var = var;
    }

    @Override
    public void run() {
        while (true) {
            try {
                Thread.sleep(2000);
            } catch (InterruptedException e1) {
                // TODO Auto-generated catch block
                e1.printStackTrace();
            }
            synchronized (var) {
                if (var.i % 2 != 0) {
                    try {
                        var.wait();
                    } catch (InterruptedException e) {
                        // TODO Auto-generated catch block
                        e.printStackTrace();
                    }
                }
                var.i++;
                System.out.println(Thread.currentThread().getName() + "   " + var.i);
                var.notify();

            }
        }

    }
}

class MyInt {
    int i = 0;

    public MyInt(int i) {
        super();
        this.i = i;
    }

    @Override
    public String toString() {
        // TODO Auto-generated method stub
        return "" + i;
    }

}

Posting an workaround without explaining the cause of the original problem is useless for readers. Please accept the above answer. — Markus Kull, Apr 12 '16 at 09:09

java wait and notify

2 Answers2