In Andrew's book, at the discussion point : Multiple Notification Objects,
page 157, there is explained the "hand-over-hand-locking".
I implemented in some other way, with only one lock (a master lock). The clients trying to lock an already locked record will wait to a Condition belonging to that master lock. In this way we will not have a new Lock object for each record and the "hand-over-hand" locking will not be necessary.
Every record will be associated with a Condition belonging to the master record.
Am I doing something wrong , is it something that I miss, what do you think ?
Thanks in advance,
On the right track but dont you think that the Condition object should belong to the individual record lock? The reason why I say that is because the master lock is used to prevent access to some kind of collection that store the records that are already locked. Another point to consider is think about why we even using a Condition - as we want to notify threads that are waiting for a specific record, if the Condition belongs to the master lock then ALL threads will be notified and then you may as well implement your locking using synchronize/wait/notify concepts instead of the ReentrantLock concept.
if the Condition belongs to the master lock then ALL threads will be notified
What I ment with " Condition belonging to that master lock " was that for each record a separate condition is used obtained with masterLock.newCondition(). In this way, only the clients waiting at a specified record will be notified when the record is unlocked.
I quote from the Condition class javadoc
Condition factors out the Object monitor methods (wait, notify and notifyAll) into distinct objects to give the effect of having multiple wait-sets per object
Maybe the example from the SCJD book wants to demonstrate this technique
of hand-over-hand locking and this is for sure not the only way to solve the problem.
Normally should be Ok, hopefully I do not miss something basic because I do not have much work experience with the "java.util.concurrent" package, but I have found this project as a good opportunity to use it.
If you are pertaining to the example in the book then this is wrong way to go as you are create a Condition object on the master, whereas you should be creating a Condition object on the record lock object. If we ask ourselves why, well associating a Condition at a record level can help you implement a way to notify waiting threads.
Consider the following code:
So here you can see the masterLock is used to gain exclusive access to the map that stores the locked records. Then once I have the recordLock we then lock the record.
well associating a Condition at a record level can help you implement a way to notify waiting threads
I'm also associating a Condition at a record level, only that every of my record Conditions is associated with the collection lock. My LockInformation
does not extend the lock, but is receives in constructor a Condition obtained from the collection lock. I want to make use of multiple wait-sets per object.
I really do not see where the catch is, why instead using multiple wait-sets per object , a Lock object for each record. One advantage that I see in Hand-over-hand locking is that time spent in the critical section of "masterLock" in lockRecord() is smaller by releasing the lock immediately afer locking the record. But is this really an issue ?
Condition - It is a much better idea and better programming technique to associate the Condition with the creator of the Lock. More so you instantiating the Condition from the "masterLock" variable and then referencing the Condition to the LockInformation. Have you considered the to make the LockInformation class of type ReentrantLock? In that way you can instantiate a new Condition object which is now attached to the owner?
I have analysed the code from the SCJD book. It can be a bad programming technique what I have done but I have found interesting to use the multiple wait-sets per object feature. The one to one relation between a lock and a condition can be used also with the old synchronization API.
lockInfo.getCondition().signalAll() - why are you using signalAll and not signal?
You are right, it is enough if I use signal here. I have introduced signalAll to use the same unlock method to unlock a record after deleting it. In that case, all clients that are waiting to lock a deleted record must be notified and they must throw the RecordNotFoundException. But for this case I use now another method with signalAll(), I have forgotten to switch back my original unlock method to signal().
[ October 24, 2008: Message edited by: Sebastian Puzon ]
I remove the lock object from the map when the record is deleted. I cannot delete it with unlock because if there are more threads waiting to lock the record, the lock/unlock mechanism does not work. So the reference to the lock stays in the map as long as the record is valid. The OutOfMemory risk exists but I in my opinion is too small to be considered, but, you have right, it has to be at least documented in the choices.txt. I also have a cache for deleted records.So, more risks. I hope that documenting them is enough.
The concept pertinent to the locking of objects is to use atomic transactions, more so simply to following the process flow of:
Lock > process > unlock
If we ask ourselves why, well quite simply we want the executing thread to finish processing the current record before any other thread can manipulate the record, thus achieving atomic transactions.
Reading your above approach it seems you consolidating the concept of the locking implementation with the concept of caching objects.
More so may I recommend you decouple the two concepts and use the locking API through the Lock > process > unlock flow execution.
1. Get and add record to map
2. Process record
3. Remove record from map (Only if thread removing the record is the owner of the lock record)