• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

is my locking sufficient

 
Marijana Grabovac
Greenhorn
Posts: 10
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I am just working on the locking/unlocking mechanism and I decided to implement both methods in Data class itself.
Since I need just writing lock, what I did is the following:
I added
private Vector lockMgr;
and my lock method is :
public void lock(int recordNo) {
synchronized(lockMgr) {
// if I am not trying to lock the whole database
// but just one record
if (recordNo != -1) {
while(lockMgr.contains(new Integer (recordNo) || lockMgr.contans(new Integer(-1))) {
lockMgr.wait();
}
}else {
// trying to lock the whole database
// cannot do it until all the locks are released
while(lockMgr.size() != 0) {
lockMgr.wait();
}
}
lockMgr.add(new Integer(recordNo));
lockMgr.notifyAll();
}
I don't have the identifier of the thread that is locking, since I made sure that no thread will try to unlock a record which was not locked by it, initially.
Here my method waits until it can obtain exclusive lock on the lockMgr object.
I am just concerned about locking the whole database. These locks would have to wait for all the records to get released, since I cannot perform database lock until any of the records is locked by some other thread.
What do you suggest? Should I keep track of the order the threads that are trying to lock records?
Thanks Maja
 
Mark Spritzler
ranger
Sheriff
Posts: 17278
6
IntelliJ IDE Mac Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Maja,
For the locking of the whole database, just assuming that the only thing calling lock that way is the server when it is trying to shutdown, I don't think you need to handle it that extensively.
You do not need to put the -1 into the "Vector"
2. I would use a different collection rather than a Vector, try HashSet instead. Vector is considered an older collection, and it is nicer to use the newer preferred collection. This also includes HashTable, which is an older collection.
3. Your notifyAll will be in your unlock method. since you don't have to notify threads waiting to lock a record that someone else locked a different record.
4. How do you guarantee that only the client that called for the lock is the one that unlocks the record?
Mark
 
John Smith
Ranch Hand
Posts: 2937
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In addition to Mark's comments, I would suggest that you use:

instead of:

Eugene.
 
Dave Teare
Ranch Hand
Posts: 80
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Eugene,
Could you please elaborate on why you prefer the synchronized in the method definition?
--Dave.
 
Dave Teare
Ranch Hand
Posts: 80
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Marijana,
Your lock(-1) Thread is going to starve to death. Considering Mark's comment:
For the locking of the whole database, just assuming that the only thing calling lock that way is the server when it is trying to shutdown, I don't think you need to handle it that extensively.

You can assume the lock(-1) Thread does not have to complete quickly, but you cannot simply wait for the lockedRecords Vector to be empty, since while it is waiting others can grab new locks, thereby forcing the lock(-1) thread to wait even longer. This could go on forever.
Again, considering Mark's comment, the server is probably trying to shutdown, so why allow new locks to be granted? I would change your code from

To this:

And at the beginning of your lock() method, check if the lockAllInProgress semaphore is set to true; if so, wait(). Of course, the unlock method will need to reset the semaphone IF AND ONLY IF the calling Thread owns the lock on -1 AND the passed in recordNumber is -1. The unlock method should also do nothing if it is called by anyone else while the semaphore is set.
Did this make sense?
As a side note, I slightly disagree with Mark on his Vector comment. IMO, Vector is not deprecated, and therefore fair game.
--Dave.
 
Dave Teare
Ranch Hand
Posts: 80
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Damn, I didn't see the OR statement in your while loop!!!

That prevents the starvation. I like that idea! Can I "borrow" it
Sorry about the bogus post!
--Dave.
 
Mark Spritzler
ranger
Sheriff
Posts: 17278
6
IntelliJ IDE Mac Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Whether you synchronize on the collection or the method makes no difference. The only difference is what Object gets locked, the HashSet or the Data Object.
Mark
 
Marijana Grabovac
Greenhorn
Posts: 10
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi All,
Thanks for the comments and suggestions.
Mark, I have a notifyAll call in my lock since I want to notify all other waiting threads that the lock on lockMgr has been released, and that they can try to lock the record they need, if it is not already locked. Different records can be locked concurrently.
The way I guarantee that the record is unlocked only by the thread that locked it, is by providing an adapter class for the Data class, and all the accesses to database have to go through that adapter. In the adapter methods I lock appropriate records, then delegate the call to Data class and then unlock the record. So, unlock is not going to get called unless lock was previously successful.
My major concern is that my lock(-1) might wait forever, since I don't have a concept of who came first. If records continue to be locked constantly, lockMgr can never become empty, thus the thread that wants to lock(-1) will never be allowed to put -1 into the lockMgr, because it has to wait for lockMgr to become empty. Is that acceptable? Should I take into account the order of the locking attempts.
I hope answered some qustions.
Thanks,
Maja
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Marijana Grabovac:
My major concern is that my lock(-1) might wait forever, since I don't have a concept of who came first. [...] Is that acceptable?
It was acceptable in my submission. Stronger even, if your clients may lock more than one record (FbN doesn't, but remember this is a generic database engine) then this is the only way to avoid deadlock unless you want to impose a specific order of locking as part of the API (e.g. "multiple locks should be acquired in order of ascending record number").
Presumably the lock(-1) feature is intended for maintenance tasks. Surely it isn't an outlandish idea that you would wait with such tasks until all other client activity had stopped.
- Peter
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic