Help : Simple IllegalMonitorStateException

[HanMing Low]

Hi,
I've stumble on a simple thread problem.
I'd like to synchronized on the block of code instead of the whole method.
However, my thread knowledge is weak and don't understand why I experience an IllegalMonitorStateException on the unlock method.
(is my code an implementation of the condition to gain the ownership of a monitor specified in the Object's notify javadoc as
"By executing the body of a synchronized statement that synchronizes on the object. ")
I tried to trace into the method call.
It seems that there is a exception in the lock, but it simply refuse to print the stack trace.
I'd greatly apprecitate any explanation.
Thanks so much.

Han Ming

public class Lock
{
    public Boolean isLocked = new Boolean(false);
    public void lock() {
        synchronized (isLocked) {
            try {
                while (isLocked.booleanValue()) {
                    isLocked.wait();
                }
                isLocked = Boolean.TRUE;
            }
            catch (IllegalMonitorStateException ie) {
                ie.printStackTrace();
            }
        }
    }
    public void unlock() {
        synchronized (isLocked) {
        isLocked = Boolean.FALSE;
        isLocked.notifyAll();
        }
    }
}

In another class that implements Runnable
    Lock lock = new Lock();
    public void run() {
        try {
            lock.lock();
            Thread.sleep(1000l);
            lock.unlock();
        } catch (Exception ex) {
            ex.printStackTrace();
        }
    }

[Rob Ross]

This one had me stumped for a bit until I realized your problem!
Boolean types are immutable. You can't change their state.
When you write
isLocked = Boolean.FAlSE;
or
isLocked = Boolean.TRUE;
you are not changing the state of the object, you are assigning a completely different object to the reference.
This becomes an issue in your unlock() method:

        synchronized (isLocked) 
{
    isLocked = Boolean.FALSE;
    try 
    {
 isLocked.notifyAll();
    }
    catch (Exception ex) 
    {
ex.printStackTrace();

System.out.println("Exception in unlock!"+ex);
    }
    
   
        }

You are synchronizing on one Boolean object, then the first thing you do is change the isLocked variable to contain a reference to a completely different Boolean object, which you do NOT have a lock on!
That's why you get the IllegalMonitorState when you call notifyAll(). You're calling it on a different object than you synchronized on.
Hope this helps!
Rob

[Shivaji Marathe]

Rob :
I tried the following code and I still get the IllegalMonitorStateException:

  
public class Lock{
public Boolean isLocked = new Boolean(false);
public void lock() {
synchronized (isLocked) {
try {
while (isLocked.booleanValue()) {
wait();
} // end if while
//isLocked = Boolean.TRUE;
}
catch (IllegalMonitorStateException ie) {
ie.printStackTrace();
}
catch (InterruptedException e) {
e.printStackTrace();
}
}
System.out.println("out of synch block");
} // end of lock
public void unlock() {
synchronized (isLocked) {
//isLocked = Boolean.FALSE;
notifyAll();
}
}
}

  
public class LockTest implements Runnable{
public void run() {
Lock lock = new Lock();
try {
lock.lock();
System.out.println("between lock and unlock");
lock.unlock();
}
catch (Exception ex) {
ex.printStackTrace();
}
}
}

Here's how I call the runnable object

 
public class Foo {
public static void main ( String args[]) {
LockTest lt = new LockTest();
Thread t = new Thread(lt);
t.start();
}
}
 

[ January 31, 2002: Message edited by: Shivaji Marathe ]

[Rob Ross]

Shivaji,
you're doing the exact same thing that I said was the problem in the first place!
You are calling wait() and notifyAll() for a different object than the one you synchronized on.
You are synchronizing on isLocked, but you are calling wait() for the Lock object. You should be writing
isLocked.wait() and
isLocked.notifyAll();

Rob

[Shivaji Marathe]

Good catch.

[HanMing Low]

Hi Rob,
Thanks for the explanation.
It really didn't strike me that this was the reason.
And it also helps in understanding other area in thread.
Thanks so much.

Han Ming