Originally posted by Andrew Monkhouse:
What I was suggesting is that as part of the deletion method, you would presumably want to ensure that no threads will remain waiting for the lock. Consider for example:
Originally posted by Reza Rahman:
I still think it is shame you are not pursuing the approach of limiting or elimiting lock deletion
Originally posted by Frans Janssen:
I feel that you'd better use the Java assert mechanism, since that demonstrates that you know how to use the language's features.
Originally posted by Reza Rahman:
Actually, using a Condition object per record will still cause an artibrarily chosen thread to wake up.
Originally posted by Reza Rahman:
You do realize, you will still need to establish an acceptable boolean condition for the await() call, putting you right back to keeping a map of records to track locks somehow, in this case all you would have accomplished is semantically replacing synchronized blocks with lock/unlock and wait-notify with await/signal. What's more, this will still keep the dilemma of when/whether to delete the conditions still active
Originally posted by Reza Rahman:
My locking implementation is ridiculously simple, boring and unoriginal.
Originally posted by Reza Rahman:
Once you crystalize your logic, it might be worthwhile to post back and put the pseudocode up to some scrutiny again.
Have some map that contains objects corresponding to locked records, say lockedRecords. This can actually be a map containing Threads as owners of the map. Thus the map can be Integer->Thread.
1. Synchronize on lockedRecords
2. A while loop with the condition that the map contains a mapping for the current record
2a.wait on lockedRecords (this releases the monitor on lockedRecords and allows other threads to access syncrhonized blocks)
3. Add the current thread to the map
4. End synchronzed block
The unlock method is synchronized on lockedRecords, and removes the mapping Integer->Thread from the map for the current record (if the current thread is the owner) and notifies all those waiting on lockedRecords.
Have some map that contains Conditions corresponding to locked records, say "conditions". Thus the map can be Integer->Condition
Have some map to store the owners. Thus the map can be Integer->Thread
Have a single ReentrantLock to control access to the maps, say "lock"
1. Lock "lock"
2. If there isn't a mapping for the current record, create a condition object for it. If there is a mapping for the current record, then retrieve the condition object
3. A while loop with the condition that the current record is in the map
3a.await on the condition object (this releases the lock on "lock" and allows other threads to lock "lock")
4. Add the condition object / current threads to the map.
5. Unlock "lock"
The unlock method locks "lock" on entry. and unlocks it on exit. In between it removes the mappings from the current record to Thread/Condition objects (if the thread is the current owner). It also signalsAll the threads waiting on this condition object
Originally posted by Andrew Monkhouse:
Exactly! And, when you unlock it again, you must ...
Originally posted by Reza Rahman:
but remember that performance is not the only relevant metric for the exam. Me, personally, would write simpler code that is more easily maintainable any day.
Originally posted by Reza Rahman:
You might also want to consider the much simpler synchronized-wait-notify model discusssed on this forum many times. It seems using "ReetrantLock" is making your life harder rather than easier...
Originally posted by Andrew Monkhouse:
Actually, I would get rid of synchronized blocks altogether, and use java.util.concurrent.locks.ReentrantLock in combination with multiple java.util.concurrent.locks.Condition - one condition for each record.
Originally posted by Andrew Monkhouse:
I was thinking more about your comment that records would be removed from the map at some point - in this case, are they removed after the deletion? Or are they left there until after every thread has locked the record / found that the record has been deleted / *** something important happens here *** / throws the appropriate exception.
Originally posted by Reza Rahman:
Sorry about the long delay. Mondays and Fridays are the worst for me.
Originally posted by Andrew Monkhouse:
Sorry for taking so long to respond - I have had a few other things keeping me busy.
Originally posted by Reza Rahman:
1. Is there a compelling reason to remove the lock objects from the collection at all?
Originally posted by Reza Rahman:
2. Is there a compelling reason not to use ReentrantLock instead?
Originally posted by Andrew Monkhouse:
Also - instead of having locks on different record keys, have you considered having different notification objects?
Originally posted by Andrew Monkhouse:
By the way - have you thought about what happens if a record is deleted in your solution?
1. Writelock the collection of locks
2. If in the collection a ReentrantLock corresponding to lockNumber doesn't exist, then create and add it.
3. write lock the ReentrantLock corresponding to lockNumber
4. Unlock the collection of locks
5. Store somewhere (details to be worked out!) that there is another thread wanting to get the lock (increase some counter by 1)
6. unlock the ReentrantLock
7. Call lock on the ReentranLock.
The unlock method I will omit, but mention that I will still have a write lock on the ReentrantLock, but I think a read lock on the collection of locks so to allow concurrent unlocks of different locks. I think this will be fine using a ConcurrentHashMap (or whatever collection I end up using).
Originally posted by Andrew Monkhouse:
I have included a section on it in The Sun Certified Java Developer Exam with J2SE 5, Second Edition
I saw this on another thread... however, I though about it then, and now, and I just can't seeOriginally posted by Andrew Monkhouse:
If your key is an Integer, then you are using an immutable object as your key. Does this have any ramifications on your design?
I thought about this one as well. My problem is in locking a record I think need to 1) retrieve the lock from the collection (or create and add it if it doesn't exist), and then 2) "lock" it.Originally posted by Andrew Monkhouse:
Do you really need to exclusively lock the entire collection of locks? Or could you have a read-only lock on the collection, and a write lock on the key for your record? What benefit would that give you?
Originally posted by Lara McCarver:
If you need to throw a RecordNotFoundException if the record is invalid, then you need to know which records are valid. I didn't see a check for that in your pseudo-code.
Originally posted by Reza Rahman:
2. Remember that corresponding lock and unlock calls could be nested to make your logic a little more crisp.
A map of Locks, (with the keys as Integers), called "locks" below
A lock of the map, called "locksLock"
In the lock(int lockNumber) method:
1. Call locksLock.lock()
2. If in locks, there isn't a lock corresponding to the lockNumber, create one and add it to the collection.
3. Retrieve the Lock from the collection corresponding to the lockNumber
4. Begin synchronized block on the retrieved Lock
5. Call locksLock.unlock()
6. Call lock() on the retrieved lock.
7. End the synchronized block on the retrieved Lock.
The unlock method has a similar structure, except that it holds the lockLock lock for the entire method, and that in the synchronized block on the retrieved lock, it not only unlocks the lock, but if there are no other threads waiting to get the lock, it removes it from the collection. The number of threads waiting to get the lock is stored internally in Lock.