Alexey Danilov
Alexey Danilov

Reputation: 311

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

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();
            }
        }
    }

Upvotes: 0

Views: 227

Answers (3)

Peter Lawrey
Peter Lawrey

Reputation: 533482

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.

Upvotes: 0

Gray
Gray

Reputation: 116858

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.

Upvotes: 1

Razvan
Razvan

Reputation: 10093

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.

Upvotes: 3

Related Questions