Multithreading in java (two threads extensively using both notify() and wait())

Question

When I try to run a following program, there is an IllegalMonitorStateException in the first call to notify () in this piece of code:

synchronized (c) {
    try {
        notify();
        ....

This throws me off a little bit: how is it possible for the code not to have a lock on an object (c) when it is already in the synchronized block, that checks for the same lock?

Please do not mind somewhat odd-looking overuse of notify() and wait(). I know there are different (and more efficient) implementations available which perform the same task, but right now I am trying to figure out why this particular one does not work.

The full code is as follows:

  class J implements Runnable {

        public static void main(String[] x) {
            Calc calc = new Calc();
            J j = new J(calc);
            Thread t1 = new Thread(j, "one");
            Thread tCalc = new Thread(calc, "Calc");
            tCalc.start();
            t1.start();
        }
        Calc c;
        public J(Calc c) {
            this.c = c;
        }

        public void run() {
            synchronized (c) {
                try {
                    notify();
                    c.wait();
                } catch (InterruptedException e) {

                }
                System.out.println(Thread.currentThread().getName() + ": x = "
                        + c.getX());
                notify();
            }
        }
    }

    class Calc implements Runnable {

        private int x;

        public int getX() {
            return x;
        }

        public void run() {
            synchronized (this) {

                for (int x = 1; x <= 11111; x *= (x + x)) {
                    this.x += x;
                    try {
                        notify();
                        this.wait();
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
                notify();
            }
        }
    }

score 3 · Answer 1 · answered Aug 02 '12 at 14:18

3

You are trying to notify a thread waiting for a different lock which you are not currently holding. The error is justified. You have a lock on the object c and then you notify other threads about the lock on the current object.

answered Aug 02 '12 at 14:18

Razvan

9,925
6
38
51

Correct- specifically, `notify();` means `this.notify();`, not `c.notify();`. You'd need to `synchronize(this)` to call `notify();` – Chris Shain Aug 02 '12 at 14:19
As far as I can see it (and somehow I know this is not right, but not completely sure why) synchronized (c) in class J and synchronized (this) in class Calc are essentially the same object locks - because class J has an instance field Calc c. In main it is clearly visible that there is only one instance if class Calc, which is passed when class J is being constructed. – Alexey Danilov Aug 02 '12 at 18:27
Yes, it's the same object but that's not the problem. The problem is that in this piece of code: synchronized (c) {try { notify(); ... you have the lock of c and then you call j.notify() which leads to the thrown exception – Razvan Aug 02 '12 at 18:36
yep, I've got it all figured out thanks to the Gray's comment below. Problem for me was that the book I read stressed on the use of notify() instead of particularObject.notify() (as it was focusing on synchronized methods rather than blocks) and these nuances were not made entirely clear for me. Thanks for clarifying! – Alexey Danilov Aug 02 '12 at 19:15

score 1 · Answer 2 · answered Aug 02 '12 at 14:20

synchronized (c) {
    try {
        notify();
        ....

You are locking on the object c but you are notifying on the object this. It is a good practice to always specify which object you are wait() or notify() on explicitly in the future. You should do:

synchronized (c) {
    try {
        c.notify();
        ....

or:

synchronized (this) {
    try {
        this.notify();
        ....

It looks like you actually mean to be dealing with 2 locks. If this is the case then you'd do something like the following.

synchronized (c) {
    try {
        synchronized (this) {
            this.notify();
        }
        c.wait();

It is important to make sure that you are always locking c first and then this otherwise you will deadlock.

Thanks Gray, now after the first line of your comment I see it clearly. I erroneously supposed that when you enter a synchronized block with a specified object as a lock any you simply call notify(), it is automatically supposed to mean thatObject.notify(), and not this.notify(). — Alexey Danilov, Aug 02 '12 at 18:32

score 0 · Answer 3 · answered Aug 02 '12 at 14:50

0

What you have is a complicated way of doing the following.

ExecutorService printer = Executors.newSingleThreadExecutor();

int x2 = 0;
for (int x = 1; x <= 11111; x *= (x + x)) {
    x2 += x;
    final int x3 = x2;
    printer.submit(new Runnable() {
        @Override
        public void run() {
            System.out.println(Thread.currentThread().getName() + ": x = " + x3);
        }
    });
}
printer.shutdown();

This would be even simpler if you were not trying to use two threads to do something which is faster/simpler to do with one.

answered Aug 02 '12 at 14:50

Peter Lawrey

525,659
79
751
1,130

Thanks for the feedback, Peter. My code example is hugely ineffective and an overkill, I know. The whole point was to better understand how notify and wait work and not to use something this monstrous in actual work. – Alexey Danilov Aug 02 '12 at 18:35
Whenever you perform a notify(), you should set a value which you check on a wait() – Peter Lawrey Aug 02 '12 at 19:40

Multithreading in java (two threads extensively using both notify() and wait())

3 Answers3