• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

record lock&unlock

 
dennis deng
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi, Guy.
Following is my record lock&unlock draft, does some one give me some advices.
//sample
Hashmap hm;
public void lock(int rec)throws IOException{
synchronized(hm)while(hm.haskey(rec)){
try{ wait(100);}
catch(InterruptException e){}
}
hm.put(rec,rec.toString());
}
public void unlock(int rec)throws IOException{
if (hm.haskey(rec)) hm.remove(rec);
}
Thx.
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Briefly --- You are using Object.wait() as if it were Thread.sleep()! Get rid of the timeout and use notifyAll() in the unlock method. Assuming this code is sitting inside some kind of lock manager class (recommended), why a synchronized block instead of a simple synchronized method? An integer is not a valid hash key. The way you are currently using the HashMap, it really wants to be a HashSet. But it will probably end up being a HashMap mapping record number to some type of client ID, right? Finally, unlock() needs synchronization and the abovementioned notifyAll().
- Peter
 
dennis deng
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thx Peter,but I must say some thing about this :
1. synchronized block instead of a simple synchronized method means i don't want to lock the whole db file once a client get access to the db . you know , synchronized one pubic method of one object means you must abtain the object lock first.
2. also, i don't synchronized the unlock method to avoid this problem and deadlock. so i can not use notifyall() method because it must in synchronized block.
3. I use RMI . I don't know what you mean about"being a HashMap mapping record number to some type of client ID"? do i need client ID? has i missed something important? I write this FBN assignment only one week, so maybe i got something wrong. Please tell me , $250 is not a small number for me .Thanks alot : )

Originally posted by Peter den Haan:
Briefly --- You are using Object.wait() as if it were Thread.sleep()! Get rid of the timeout and use notifyAll() in the unlock method. Assuming this code is sitting inside some kind of lock manager class (recommended), why a synchronized block instead of a simple synchronized method? An integer is not a valid hash key. The way you are currently using the HashMap, it really wants to be a HashSet. But it will probably end up being a HashMap mapping record number to some type of client ID, right? Finally, unlock() needs synchronization and the abovementioned notifyAll().
- Peter

 
Mark Spritzler
ranger
Sheriff
Posts: 17278
6
IntelliJ IDE Mac Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You need some sort of clientID. You cannot have a user unlock a record that they never locked themselves. That is in the requirement. Yes you do want to synchronize in unlock and call NotifyAll. For deadlocks, that is determined on when you get the lock, is this at the time of modifing the record of a flight they are booking, in which case the lock should be there for a really short time, in my case milliseconds. Or do you lock the record when the user queries, and hasn't amde up their mind on booking or not, then you need some other timeout, not wait(100).
Also look into the Unreferenced interface, this might help a little for you
Mark
 
dennis deng
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thx, Mark..
1.But you know, I look after the record access myself. such as this sequence: lock -modify-unlock. so client A must first lock record(abtain the hashmap object lock) , do something and unlock this record, so if record #1 is lock by client A , client B must first abtain the record #1 lock then can unlock, it is impossible! and ,in programming, I also sequence access record. so client can not unlock one record before lock it.
2.do i need lock record when user queries ? i am not sure this, would you please tell me the reason? thanks alot.
As you know , i work on this assignment only one week .I must miss something important. please tell me.. thans alot.
dennis

[Also look into the Unreferenced interface, this might help a little for you
Mark[/B]
 
dennis deng
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thx, Mark..
1.But you know, I look after the record access myself. such as this sequence: lock -modify-unlock. so client A must first lock record(abtain the hashmap object lock) , do something and unlock this record, so if record #1 is lock by client A , client B must first abtain the record #1 lock then can unlock, it is impossible! and ,in programming, I also sequence access record. so client can not unlock one record before lock it.
2.do i need lock record when user queries ? i am not sure this, would you please tell me the reason? thanks alot.
As you know , i work on this assignment only one week .I must miss something important. please tell me.. thans alot.
dennis

[Also look into the Unreferenced interface, this might help a little for you
Mark[/B]
 
Mark Spritzler
ranger
Sheriff
Posts: 17278
6
IntelliJ IDE Mac Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
No you won't lock for queries.
The reason why you want to synchronize in the unlock method, is simply because that is what they are expecting to see. Even though you and I know that the time between the locking modifying and unlocking by one client will not fail, it is still being complete, or should I say erring on the side of safe.
You cannot guarantee that the client will get to unlock the record before someone else tries to lock and unlock it. It is just not clean or complete.
Mark
 
dennis deng
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thx , Mark.. here is some reason i can not synchronized unlock method.
1. if you study my sample code , you will find the Hashmap contain all locked record id. client abtain the lock of the Hashmap object means he will lock the whole DB .so I must put wait(millisecond) in lock method to allow concurrent access DB.
2. I can not synchronized the unlock method , see this scenario: client A get HashMap object lock and lock record #1 and return from sychronized lock method .the record Id #1 will stored in the HashMap. then client A will do something like modifying. at this time client B abtain the lock of the HashMap object ,get into the sychronized lock method and want to lock record #1, as you know , the record id is already in HashMap object , client B must loop and wait for client A release the lock of record #1. if i also sychronized the unlock method , the client A can not abtain the HashMap object lock (client B hold ), so client A can not get into the unlock method block and can not unlock record #1, this is deadlock!! am I wrong ?
3. if sychronized whole lock or unlock method instead of synchronized method block , it will lock whole DB , do i right?
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Mark Spritzler:
The reason why you want to synchronize in the unlock method, is simply because that is what they are expecting to see.
That is not the reason. The reason is that if you don't synchronize unlock() the code is badly broken. If another thread is updating the HashMap the very moment unlock() is being called,
  • The map may be in an internally inconsistent state, and hm.hasKey(rec) may return an incorrect value.
  • The hm.remove(rec) may interfere with the other update and completely mess up the internal structure of the map.
  • Even when there is no other update underway, Java gives you no guarantee that the state of the HashMap seen by the unlock() method is actually up to date, unless you synchronize. This is a subtle and under-appreciated point of the Java language. You'll need pretty exotic hardware to actually see this, mind.
  • Dennis, in your argument why synchronizing unlock() will cause a deadlock you forget one thing: wait() releases the monitor lock. If you'd have used sleep() it would have been different. But I can only reiterate my point that you should not use wait() without using notify() or notifyAll(); that's what it's for. As it stands I would expect you'd be penalized for lack of thread safety and improprer use of API.
    If I understand your code correctly, there is a 1:1 relationship between the HashMap and the object that has the lock() and unlock() methods. Synchronizing the methods is therefore equivalent to blocks synchronizing on the HashMap, but a a lot clearer.
    You say "I look after the record access myself". This is not true. Remember that you are asked to code this database for reuse - presumably, not necessarily by yourself. This means the API should be a bit more robust than you would require of one-off code. Besides, my copy of the assignment (2 years ago) contained some explicit requirements regarding lock() and unlock().
    One week is an unrealistically short time for this project unless you are very comfortable with the ground covered. I'm sure we will all do our best to help you but try to plan some contingency if you can. All the best,
    - Peter
    [This message has been edited by Peter den Haan (edited November 09, 2001).]
 
dennis deng
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Peter den Haan:
[B]
Originally posted by Mark Spritzler:
If I understand your code correctly, there is a 1:1 relationship between the HashMap and the object that has the lock() and unlock() methods. Synchronizing the methods is therefore equivalent to blocks synchronizing on the HashMap, but a a lot clearer.
hi,Peter , it is very kind of you,thanks alot . But what exact you mean above words? would you please explain it to me in detail ? also ,can i the modify lock/unlock signature ? you know , there is no synchronized modifier in lock/unlock assigment . BTW , if We abtain the lock of the object that has the lock() and unlock()method, does it mean we lock the whole DB ?
 
Martin Habicht
Greenhorn
Posts: 17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
hi all,
let's say I have lock() and unlock() mehtods synchronized (Peter and Mark I completely agree that this is needed). From where do I call them? From modify(), delete() and add()? As of now they are all synchronized themself (acutally on the Data instance, of which I have only one). So if I call lock() and unlock() within the methodbody of modify(), delete() and add(), those 3 don't need to be synchronized and probably shouldn't be. Is this correct?
There are two underlying private methods (readRecord() and writeRecord()), which are also synchronized, but I better keep them sync - for safety?
Well, I usually use locks in DB to isolate transactions, but as far as I see, we only use atomic operations, no transactions, so locks are just a nice thing to make the db extenable for future purpose, right?!?

I appreciate your comment and have a nice weekend :-)
-Martin
 
Mark Spritzler
ranger
Sheriff
Posts: 17278
6
IntelliJ IDE Mac Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I think you lock the record before you call modify, add or delete, not the other way aroud. And the calls to modify, add or delete are handled by a user of the data class. So that user will first lock the record, then call the other data methods. then call unlock.
That is the basic principle behind it.
Mark
 
Martin Habicht
Greenhorn
Posts: 17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
ok, i understand what you mean, but...

if the user calls lock(), modify(), unlock() why does modify() needs to by sync? To make sure local variables won't get screwed?

And if modify(), add() and delete() are synchronized, why do we need locks? We don't have transactions, just atomic operations. So one user enters the synchronized modify() and until the method returns, nobody can call a method of this instance of Data. Or do I miss something?!?

-Martin
 
Mark Spritzler
ranger
Sheriff
Posts: 17278
6
IntelliJ IDE Mac Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I believe that when a method is synchronized, only one thread can call that method at a time, but it doesn't stop other threds from calling other methods on that class.
So when you lock and unlock, what is really important is synchronizing on the Collection class that is storing all user locks.
Imagine this. I am user A, I lock record # 1
user B comes along and wants to also lock record # 1.
Let's look at some scenarios
1. They both try to lock record #1 at the same time. only one thread/user will get the lock, the other user will have to wait.
When the first user is done and unlocks the record, it notifies all the threads waiting. So the first user can notify the second, hey I'm done, go check the collection to see if what I just unlocked was the same record you want to get. If it is then user b can now get the lock.
Without synchronizing, you can have no guarantees on who gets what, and there might even be a point where the information one user is getting is invalid data.
Actually that above example kind of explains all the scenarios.
Mark
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Mark Spritzler:
I believe that when a method is synchronized, only one thread can call that method at a time, but it doesn't stop other threds from calling other methods on that class.
It stops other threads from calling any synchronized method on the object. It works on a per-object basis, not a per-method basis. Actually it is equivalent to a synchronized(this) block.
If every instance of the class has its own copy of the collection (this is what I meant with a 1:1 relationship), it is not necessary to synchronize on the collection; just use synchronized methods, they are easier to take in.
Martin, you asked "if the user calls lock(), modify(), unlock() why does modify() needs to by sync?" Well, because clients can still call these methods without locking, or call other methods such as find() that do not require locking in the first place. You cannot allow multiple threads to access your RandomAccessFile, even if only one of them is writing to it. If you carefully examine how Data works internally you'll discover quite a loopholes that open up if a modifying thread is allowed to interact with a reading thread like that.
- Peter
 
Martin Habicht
Greenhorn
Posts: 17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
first of all, thanks for your detailed and knowledgeable replays.
i now understand, that modify() needs to be synchronized. just like all the other data reading/writing methods.

but im my design I use a single instance of Data, which is shared by all clients. (remote and local - there are interfaces and wrapper classes in between, but in the end the single instance of Data will handle all requests). Or just say Data is a Singleton. So in this situation, only one thread can perform an operation (which are now all sync) at a time. All other threads need to wait until they get the monitor on the Data instance.
a) do I need to call wait() and notifyAll() in a loop or doesen't it already works in a way, that thread_2 waits automatically until he can get the monitor?
b) what for do I need locks in this situation? (apart from the sun requirement)

thanks...
Martin

PS: did you specialists knew that synchronize(this){method body} and syncronized method() and has the same effect, but gets compiled to different bytecode? But that's a different issue...
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Martin Habicht:
but im my design I use a single instance of Data, which is shared by all clients. [...] Or just say Data is a Singleton.
Heh. You just hit one of my hobby horses - yes, you use a single instance of Data, just beware of imposing a Singleton pattern on it. Remember, you're coding for reuse, and the next project that comes along might have a need for multiple tables.
a) do I need to call wait() and notifyAll() in a loop or doesen't it already works in a way, that thread_2 waits automatically until he can get the monitor?
When it comes to monitor locks there's nothing you need to do apart from sticking the word "synchronized" in a few places. But sometimes the thread gets its monitor lock, goes into a synchronized method and then discovers that some condition isn't met. For instance, the method in question is lock() and another client already owns the lock in question. That's where wait()/notify() comes in.
b) what for do I need locks in this situation? (apart from the sun requirement)
Record locks work a bit like the monitor lock, but on a completely different level. They lock records, not objects. They live across machines and method calls and aren't tied up with code structure like a synchronized block is; instead, you call lock() and unlock() methods. You need locks to prevent race conditions, i.e. multiple clients from booking the same seat at the same time.
PS: did you specialists knew that synchronize(this){method body} and syncronized method() and has the same effect, but gets compiled to different bytecode? But that's a different issue...
Yes. A synchronized method is slightly more efficient because there's a special bytecode for it.
- Peter
 
Noor, Omar
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Very good forum. I found it today.
I just started the developer's assignment; almost done the database implementation by modifying the Data class. When it came to lock/unlock method implementation, I found the easiest way to associate a lock with a specific connection/client is pass extra parameter to lock/unlock. This raised a suspision in me if that is acceptable because the assingnment requriements gives lock/unlock method with only one parameter. Did any of you did this and get away with it?
As extention of this question, I thought it would be good idea if I safeguard the database from clients that might call one of those db modifying methods(Delete, modify,etc) without obtaining the lock first. To do this I have to modify those methods to have them checked the record and wait on the record if it's locked by another thread. Is this also acceptable?.
I know the cleanest way is to trust the clients to fulfill the prerequite by obtaining the "write" lock before calling those methods.
I will appreciate your opinion on this.
Peter
I agree most of what you said about the need for synchronization. With RandomAccessFile, even the readOnly methods must be synchronized because they manipulate the filepointer. In fact I think any method that accesses the database through RandomAccessFile must be synchronized.
 
dennis deng
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
hi, peter,
I have only one instance of the DB class which as rmi database server.do you think synchronzie on the collection is same as synchronize methods? BTW, i have revise my lock code as following, can you give me some advise?
//lock()
//if whole db is locked, DBlock=false
private boolean DBLock=true;
private Vector v=new Vector();
public void lock(int record)throws IOException{
synchronized(v){
while((v.contains(new Integer(record)))| |(v.contains(new Integer(-1)))| |(DBLock==false)){
try{
v.wait();
}
catch(InterruptedException e){
throw new IOException("lock failed");
}
}
while(DBLock){
while((v.size()>0)&(record==-1)){
DBLock=false;
try{
v.wait();
}
catch(Exception b){}
}
v.add(new Integer(record));
break;
}
}
}
Originally posted by Peter den Haan:
Originally posted by Mark Spritzler:
[b]If every instance of the class has its own copy of the collection (this is what I meant with a 1:1 relationship), it is not necessary to synchronize on the collection; just use synchronized methods, they are easier to take in.
- Peter

 
chillspring
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
public synchronized void lock(int record) throws IOException {
try {
while ( (record == -1 && m_locked.size() > 0 )
| | m_locked.contains(new Integer(record))
| | m_locked.contains(new Integer(-1)) )
wait();
m_locked.addElement(new Integer(record));
notifyAll();
} catch (InterruptedException ex) {
throw new IOException("Lock operate was interrupted.");
}
}

are these code is right?
thx!
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Noor, Omar: You can actually fulfil all requirements without modifying the lock() and unlock() signature. But I've seen plenty of developers who added an argument to lock() and unlock(), and passed fine. Basically, a lot of things seem to be acceptable as long as you are able to give a cogent argument why you did it.
There is no requirement to safeguard against clients who don't lock before modifying. Having said that, I agree that it would be a good idea and I personally did enforce it (by doing an implicit lock & unlock when no lock had been acquired).
Whether the Data class is the right place to implement locking in the first place is debatable.
Dennis: Given that you have only one copy of everything (collection, db class, server class) it doesn't matter what you synchronize on - as long as everything synchronizes on the same thing Synchronized methods are easier to read though IMHO.
The data structure you've chosen to contain locks (Vector) might be inappropriate though. Not only does it needlessly add its own synchronization, the contains() and remove() methods have O(n) performance (where n is the number of outstanding locks), which is only acceptable if you are optimistic about the number of locks outstanding at any given time. For FBN that optimism is fine, but you are asked to code for re-use. Replace the Vector by a HashSet, which lacks synchronization, has O(1) performance for all relevant operations, and is a better expression of the kind of collection your locks are (mathematically speaking, they really are a set and certainly not a list; in plain English, you can only hand out one copy of each lock and you don't care a hoot about the order of the locks.)
You've got some duplicate information: a database lock is both indicated by DBLock and a -1 record lock; can't you eliminate one of them? The database lock code is broken, I think - it looks like no database lock will be set unless there are other outstanding locks, or am I misreading the code? The & wants to be && also.
Finally, pull your "new Integer()" statements out of the loop, no need to create new objects every time someone calls notifyAll().
Chillspring: why the notifyAll()? And please reregister with your full name; see our naming policy. Thanks.
- Peter
 
dennis deng
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
hi, peter , thanks alot .you are my teacher now ; ) how about following coding :
//lock method
private HashSet hs=new HashSet();
public synchronized void lock(int record) throws IOException {
if(record < -2 | | record > getRecordCount() | | record == 0){
throw new IOException("The record position is invalid.");
}
else
{ Integer record_obj=new Integer(record);
while ( (record == -1 && hs.size() > 0 )| | hs.contains(record_obj)| | hs.contains(new Integer(-1))){
try{
wait();
}catch (InterruptedException e) {
throw new IOException("Lock was failed!");
}
}
hs.add(record_obj);
}
}
//unlock method
public synchronized void unlock(int record)throws IOException{
if(record < -2 | | record > getRecordCount() | | record == 0){
throw new IOException("The record position is invalid.");
}
else {
Integer record_obj=new Integer(record);
if (hs.contains(record_obj)) {
hs.remove(record_obj);
notifyAll();
}
}
}
Originally posted by Peter den Haan:
Dennis: Given that you have only one copy of everything (collection, db class, server class) it doesn't matter what you synchronize on - as long as everything synchronizes on the same thing Synchronized methods are easier to read though IMHO.
The data structure you've chosen to contain locks (Vector) might be inappropriate though. Not only does it needlessly add its own synchronization, the contains() and remove() methods have O(n) performance (where n is the number of outstanding locks), which is only acceptable if you are optimistic about the number of locks outstanding at any given time. For FBN that optimism is fine, but you are asked to code for re-use. Replace the Vector by a HashSet, which lacks synchronization, has O(1) performance for all relevant operations, and is a better expression of the kind of collection your locks are (mathematically speaking, they really are a set and certainly not a list; in plain English, you can only hand out one copy of each lock and you don't care a hoot about the order of the locks.)
You've got some duplicate information: a database lock is both indicated by DBLock and a -1 record lock; can't you eliminate one of them? The database lock code is broken, I think - it looks like no database lock will be set unless there are other outstanding locks, or am I misreading the code? The & wants to be && also.
Finally, pull your "new Integer()" statements out of the loop, no need to create new objects every time someone calls notifyAll().
- Peter

 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Dennis, looks good (I think: so test!) just one very minor point: I would suggest addingReplace all references to "-1" by ones to "DATABASE_LOCK"; replace "< -2" (should've read <=) by "< DATABASE_LOCK" and replace "new Integer(-1)" by "DATABASE_LOCK_INTEGER". Clearer, and you save some object churn with the last replacement.
- Peter
 
dennis deng
Ranch Hand
Posts: 42
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks peter, I have learned alot of things from you. thanks alot.
dennis
Originally posted by Peter den Haan:
Dennis, looks good (I think: so test!) just one very minor point:
- Peter

 
Leo Xie
Greenhorn
Posts: 8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In my assignment, it says Record locking must be implemented using the methods public void lock(ing) and public void unlock(int).
It seams that I should NOT modify the methods' formats. I think I shoud use synchronized block in lock(int) method in stead of synchronizing both of the two methods. Am I right?
Thanks.
[This message has been edited by Leo Xie (edited November 15, 2001).]
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What Sun is giving you are the required method signatures. Synchronization is not part of the signature; it's an implementation detail. You are certainly not expected to insert a synchronized block just to avoid adding the "synchronized" keyword in front of the method.
- Peter
 
Matthew Comer
Ranch Hand
Posts: 37
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Not at all meaning to be critical, but it seems that a number of people are confused about syncronization, thread safety and thread safety issues, a Java object's monitor, when the monitor is obtained and given up, etc.
Please let me recommend an extremely informative series of articles on the subject:
http://www.javaworld.com/javaworld/jw-09-1998/jw-09-threads.html?
Trust me, this series will get you up to speed...
Matt
 
Noor, Omar
Greenhorn
Posts: 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you Peter for the answer
I know I can implement locking mechanism in a level above the Data class or in another class which has aggregate association with Data. But I thought for this kind of application, it's simpler and easier to have a local HashMap member in Data class and make lock/unlock have signature with two arguements. There is also another point here; according to the assignment requirement, lock/unlock has to be implemented as part of Data API. In addition to this, unlock has to know who holds the lock on the given record. If the lock is hold by different client than the one calling unlock()now, then unlock() has to ignore that call. It should not allow a mistaken caller to unlock a record that it did not locked priori. To determine this, using currentThread reference, I think, is not good idea because of RMI or if the server wants to free resources of dead client thread. Given that requirements, how unlock() with only the record number parameter will know who is unlocking this record??
As Dennis code demonstrates this, unlock() will always try to unlock the given record no matter who holds locks of that record. and I think that does no fulfill the requirement!!

 
Rasika Chitnis
Ranch Hand
Posts: 131
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
By making lock and unlock methods synchronized, all other threads are prevented from executing any other synchronized method in Data class.
For example, while one thread is excuting lock method, no other thread will be able to search flights because getRecord method (which is synchronized) can not be executed. I do not think we want this to happen.
Whereas if lock and unlock methods have synchronized blocks in them (synchronizing on HashSet containing locks), other threads can still do a search. And the data integrity is still maintained by having 'book flight' follow the sequence lock -> readRecord -> modify -? unlock.
I think there is a benefit of having synchronized block instead of synchronized method.
Any thoughts ?
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Noor, Omar:
according to the assignment requirement, lock/unlock has to be implemented as part of Data API.
Mmmm... it doesn't say it has to be implemented in Data itself.
To determine [client ID], using currentThread reference, I think, is not good idea because of RMI or if the server wants to free resources of dead client thread. Given that requirements, how unlock() with only the record number parameter will know who is unlocking this record??
See here for some hints. There's plenty of discussion if you trail through past threads too.
As Dennis code demonstrates this [...] I think that does no fulfill the requirement!!
True, and it was touched upon twice earlier in this thread. However, experience shows that you can pass without satisfying this requirement, and Dennis is rather pressed for time.
- Peter
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Rasika Chitnis:
For example, while one thread is excuting lock method, no other thread will be able to search flights because getRecord method (which is synchronized) can not be executed. I do not think we want this to happen. [...] I think there is a benefit of having synchronized block instead of synchronized method.
Your point is a valid one. However, seeing that the lock() and unlock() code can be expected to hold locks only very briefly, don't expect any great gains in concurrency. And if your design is clean and you implement your locking code in a separate class, the synchronization will be properly factored out without you really having to think about it.
- Peter
 
Rasika Chitnis
Ranch Hand
Posts: 131
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Peter, thanks for you reply. There is one more issue with protecting data (in the flat file) in local mode. If I open multiple DOS windows or xterm windows and run the client concurrently in local mode, synchronization does not help, right ? because the client in every window is running in it's own JVM, has its own instance of Data class but ultimately all clients use same file in the file system. How can file data be protected from corrupting ?
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You could use a lock file to prevent this (use File.createNewFile(), e.g. File.createNewFile(databaseFile + ".lock")). Personally I didn't implement it, I thought it was getting a bit over the top. But you can easily argue that this was a bad decision.
- Peter
 
Rasika Chitnis
Ranch Hand
Posts: 131
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks very much, Peter.
SUN documentation on "File" class says that "This method (i.e. createNewFile), in combination with the deleteOnExit() method, can therefore serve as the basis for a simple but reliable cooperative file-locking protocol."
But we are not creating a new database file for each client in this assignment, so this is really not that useful in this case.
Thanks again for your suggestion.
 
Rasika Chitnis
Ranch Hand
Posts: 131
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Another issue :
If for some reason unlock method can not unlock a record, should it throw an exception ? If yes, should the modified seats be displayed to the user ? The point is, even if unlock fails seats have been already modified in the database and so it makes sense to display new seats to the user. In other words the exception thrown by unlock method should not be propogated to the user. In that case, what good it does to throw an exception ? I think a thrown exception could have been used to log it into a file in a real application, but in this project there is no error log file.
On the flip side, subsequent users will fail to seek a lock on the locked record. But that will happen anyway, regardless of whether unlock method throws an exception or not.
Anybody considered this issue before ??
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Rasika Chitnis:
[...] But we are not creating a new database file for each client in this assignment, so this is really not that useful in this case.
Apologies for being unclear. The lock file would certainly not be the database file! The problem we're trying to solve is, we need a foolproof way of claiming ownership of the database file even if two different client processes are started simultaneously. One popular way of doing this, and just about the only reasonable way that Java gives you access to, is using lock files. To claim ownership of a file (or set of files), say "db.db", you create an accompanying lock file, say "db.lck". If you succeed, you own the database. If the lock file already existed, you don't. The atomicity of createNewFile() is important, it means that you cannot get race conditions when two clients try to claim the database simultaneously. As long as these clients agree on the lock file to use Note that the lock file does not need to contain any data, and has nothing to do with record locks.
You see now that a lock file is quite useful even when you're not creating a new database file.
- Peter
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Rasika Chitnis:
If for some reason unlock method can not unlock a record, should it throw an exception ?
What conceivable reasons could there be why this would happen?
- Peter
 
Rasika Chitnis
Ranch Hand
Posts: 131
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Peter, thanks for the reply.
One of the possible reason could be record number parameter is invalid.
Rasika
 
Bisi Adedokun
Greenhorn
Posts: 7
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Below is my first shot at a lock manager class for lock and unluck. Everyone please critique/advise/comment as necessary:
public class LockManager {
private final static int DATABASELOCK = -1;
private final static java.lang.Integer DATABASELOCKOBJ = new Integer(DATABASELOCK);
private java.util.Map lock = new HashMap();
private int databaseLock = 0;
public synchronized void lock(int recno) throws IOException {
Integer recnoObj = new Integer(recno);
try {
if (recno == 0 | | recno <= -2)<br /> throw new IOException("Invalid record number position");<br /> if (recno == DATABASELOCK) {<br /> // database lock required. Check to see if it's already locked<br /> if (databaseLock > 0) {
throw new IOException("database is locked");
} else { // database has not been locked, lock it!
wait();
lock.put(DATABASELOCKOBJ, DATABASELOCKOBJ.toString());
databaseLock++;
}
} else { // record lock required
wait();
lock.put(recnoObj, recnoObj.toString());
}
} catch (InterruptedException ie) {
} catch (IOException e) {
System.err.println("recno number: " + recno + " not found");
}
}
public synchronized void unLock(int recno) throws IOException {
Integer recnoObj = new Integer(recno);
try {
if (recno == 0 | | recno <= -2)
throw new IOException("Invalid record number position");
if (recno == DATABASELOCK) {
if (lock.containsKey(DATABASELOCKOBJ)) {
lock.remove(DATABASELOCKOBJ);
databaseLock--;
}
} else {
if (lock.containsKey(recnoObj)) {
lock.remove(recnoObj);
}
}
notifyAll();
} catch (IOException e) {
System.err.println("recno number: " + recno + " not found");
}
}
}
 
It is sorta covered in the JavaRanch Style Guide.
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic