aspose file tools*
The moose likes Developer Certification (SCJD/OCMJD) and the fly likes IllegalMonitorStateException Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of EJB 3 in Action this week in the EJB and other Java EE Technologies forum!
JavaRanch » Java Forums » Certification » Developer Certification (SCJD/OCMJD)
Bookmark "IllegalMonitorStateException" Watch "IllegalMonitorStateException" New topic
Author

IllegalMonitorStateException

David Reck
Ranch Hand

Joined: May 25, 2001
Posts: 37
This is the basic structure of my lock and unlock method:

r_lock is a Vector. Sometimes when I try to book seats on the same flight from multiple clients with requests that will exceed the number of seats available (all requests made at the time) I get an IllegalMonitorStateException. Should I just catch IllegalMonitorStateException in my booking exception and consider it a request that failed because there were not enough seats availabe? I have also tried r_lock.wait() instead of wait(), but that gives me messages saying all booking requests completed successfully even when there were do few seats in the database. What am I doing wrong?
[Added CODE tags to make code more readable; please use these next time -- PdH]
[This message has been edited by Peter den Haan (edited July 20, 2001).]
Trevor Dunn
Ranch Hand

Joined: Jun 13, 2001
Posts: 84
The illegalMonitorStateException means that a thread is trying to either wait or notify on an objects monitor without actually owning the monitor. I would not ignore this, it means there is something wrong with your locking mechanism.
I have a similar design to yours and used to get that problem, but not anymore onec I dixed my synchronization
Trevor
David Reck
Ranch Hand

Joined: May 25, 2001
Posts: 37
Isn't it required to sync on the Vector to keep multiple threads from accessing it at the same time?
Trevor Dunn
Ranch Hand

Joined: Jun 13, 2001
Posts: 84
Vectors are already synchronized by default. I do not know if the markers will care that you used Vector, but just in case they do you should probably use a synchronized ArrayList
ArrayList locks = Collections.synchronizedList(new ArrayList());
Look at the source code for the above example, that is how I synchronized mine
Trevor
Peter den Haan
author
Ranch Hand

Joined: Apr 20, 2000
Posts: 3252
Seconding Trevor - you're getting an IllegalMonitorStateException because you are wait()ing on <code>this</code> instead of r_lock. You don't own the monitor lock on <code>this</code>. Please read the javadoc for Object.wait().
The logic of the code you posted seems sound otherwise. Your booking problem is likely to originate elsewhere.
I disagree with Trevor's last post though. You should not be using a synchronized ArrayList. It is clear from the code you posted that the application logic requires you to use synchronized blocks (or methods) explicitly. The additional synchronization in Vector or Collections.synchronizedList() buys you nothing but unnecessary overhead.
- Peter

[This message has been edited by Peter den Haan (edited July 20, 2001).]
David Reck
Ranch Hand

Joined: May 25, 2001
Posts: 37
Peter,
I think you are right about the why I am getting the exception. I am going to change my lock method to call wait on the Vector, but I don't see how the booking problem can be occuring. I am getting false positive responses meaning that if 3 people make booking requests at the same time, in which 2 should be successful and 1 should fail they all get positive responses returned, but the database updates are correcet in that only 2 requests are actually performed. Below is an example of my booking logic see if you can tell where I might be off.
try {
//request a lock on the record
//verify there are enough seats available in db
//update record in db
//display successful update message
}
catch (IOException ioe) {
//error handling
}
catch (DatabaseException db) {
//error handling
}
finally {
//unlock record
}


[This message has been edited by David Reck (edited July 22, 2001).]
Peter den Haan
author
Ranch Hand

Joined: Apr 20, 2000
Posts: 3252
Two questions.
//verify there are enough seats available in db
This does include rereading the record from the database, doesn't it?
Also, although I don't know how significant it is:Shouldn't that read "record" instead of "currentRecord"?
If you can't isolate the problem, write a simple test program that acquires locks and prints messages. Stick in a few sleep()s. Run it twice. Shows you exactly what is happening.
- Peter
David Reck
Ranch Hand

Joined: May 25, 2001
Posts: 37
Peter,
//verify there are enough seats available in db
This does include rereading the record from the database, doesn't it?

Answer: Yes it does. I am doing that to make sure there were no additional seats taken between the time the first read is done on available seats and the time it takes to obtain the lock on the record. Is that an issue?
Shouldn't that read "record" instead of "currentRecord"?
It says currentRecord because of some logic for the -1 locking the whole database handling. It doesn't make since because I cut all of that logic out when I posted the message. It can be though of as the "record" and not "currentRecord".
As for your suggestion about testing the problem. It only occurs when the requests for seats are made for the same record at the same time. Does adding the sleeps to the method make the requests wait on each other? Should I put the sleeps in the booking method or the lock method?
Thanks for your help,
David

[This message has been edited by David Reck (edited July 22, 2001).]
Peter den Haan
author
Ranch Hand

Joined: Apr 20, 2000
Posts: 3252
I am doing that to make sure there were no additional seats taken between the time the first read is done on available seats and the time it takes to obtain the lock on the record. Is that an issue?
Not at all; to the contrary, that's exactly the way it should be done. Lock, reread, modify, write, unlock.
What I was thinking of re: testing is a simple client program that does:
print message
lock record X
print message
sleep(random)
print message
modify record X
print message
unlock record X
print message
sleep(random)
This way, you can run two or three at the same time and observe at your leisure what is happening.
- Peter
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: IllegalMonitorStateException
 
Similar Threads
IllegalMonitorStateException
Design Questions
Lock and Unlock Design Question
Locking not working properly in seperate VMs
Locking the whole Data base if -1 see code????