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


Win a copy of Spring in Action this week in the Spring forum!
JavaRanch » Java Forums » Certification » Developer Certification (SCJD/OCMJD)
Bookmark "Synchronized Lock" Watch "Synchronized Lock" New topic
Author

Synchronized Lock

Mark Spritzler
ranger
Sheriff

Joined: Feb 05, 2001
Posts: 17257
    
    6

OK, so how many times has this question been asked. But I still can't find a good answer that I like. So I am posting my lock code in the Data class, and the RemoteDataAccess class. I have tested two concurrent clients, and had one lock record #3 and loop for a few seconds, then I had the second client try to lock the same record, it waited until the first class unlocked the record before it could get a lock.
The only problem, is I have a while loop, instead of a wait()/notify(). Is this wrong?
*********lock method in my data class
public void lock(int record) throws DatabaseException{
while(lockedRecords.indexOf(new Integer(record)) < NOT_IN_LOCKED_RECORDS){}
lockedRecords.add(new Integer(record));
System.out.println(lockedRecords.toString());
}
}

******lock code in my RemoteDataAccess class
public void lock(int record) throws RemoteException, DatabaseException{
int recordIndex;
if (record == LOCK_ALL_RECORDS){
for(int loopCount = 0; loopCount < getRecordCount(); loopCount++){
lock(loopCount);
}
recordIndex = lockedRecordIndexInVector(record);
} else {
recordIndex = lockedRecordIndexInVector(record);
if (recordIndex == NOT_IN_LOCKED_RECORD){
FBNDatabase.lock(record);
lockedRecords.add(new Integer(record));
}
}
}

lockedRecords is a Vector.
Is my code threadsafe, or a big risk?
Thanks
Mark


Perfect World Programming, LLC - Two Laptop Bag - Tube Organizer
How to Ask Questions the Smart Way FAQ
Peter den Haan
author
Ranch Hand

Joined: Apr 20, 2000
Posts: 3252
Originally posted by Mark Spritzler:
The only problem, is I have a while loop, instead of a wait()/notify(). Is this wrong?
Quite simply, yes. On machines without pre-emptive threads, your implementation will sooner or later hang because one thread keeps on hogging the CPU (you may fix that by incorporating a yield() in the while loop). Furthermore, it wastes large amounts of perfectly good CPU time by sitting in a tight loop like that.
Is my code threadsafe, or a big risk?
The code is not threadsafe. Take another look at it:
There are obvious problems here, for a start the braces don't match up, but I assume you meant<br /> Because there is no synchronization, nothing prevents another thread from grabbing the lock just between your while and your add, causing you to grant the same lock twice. The risk may be small but it is a major bug. You may be selling the same seat twice.
There are other, less important problems.
Using contains() rather than indexOf() would be a lot clearer.
The use of indexOf tells me you are using some type of List, probably an ArrayList or a Vector. Characteristic of these are that they are ordered and allow duplicate elements. However, for the locks you are not interested in the order and you do not want any duplicate elements! What you need is not a List, but a Set, probably a HashSet. It is no coincidence that it will be a lot faster too (O(1) instead of O(n)).
Finally, the while() loop above will generate huge amounts of object churn because it keeps on creating Integer objects; this will flood the garbage collector and greatly slow down your server. Create a single Integer at the start of the method and keep on using that.
- Peter

[This message has been edited by Peter den Haan (edited October 01, 2001).]
Mark Spritzler
ranger
Sheriff

Joined: Feb 05, 2001
Posts: 17257
    
    6

Again Peter you come through in the clutch.
I was reading in the O-Reilly book "Learning Java" and I read about thread and wait and notify. I think I will synchronize the "Vector", if it remains a Vector. Then in the loop, it will have a wait(). In the unlock method I will synchronize the same object, and when it is done unlocking it will call notifyAll().
In this case can I keep the Vector. Yes I know it allows duplicate values, but all it takes is one object, the Integer of the record number, whereas a HashMap will need two objects, the Key and the Object. However, thinking about it, Key is the Integer.toString(), and the object is the Integer, and then I can just use contains(TheInteger).
Thanks again Peter
Mark
Mark Spritzler
ranger
Sheriff

Joined: Feb 05, 2001
Posts: 17257
    
    6

One more thanks Peter. I made the changes and used a HashSet instead, and I tested it with two JVM's A & B trying to lock the same record, A first runs and gets the lock on the record, it then loops for a few seconds, in which I run B, which is trying to lock the record for the record that is locked by A. It waits, and when A unlocks and it notifies all, then B gets the lock. All really cool, and it only took me ten minutes to change what I had, to what you where suggesting.
HashSet is much easier with less code.
Thanks
Mark
 
Consider Paul's rocket mass heater.
 
subject: Synchronized Lock