Nested Locks

[Javini Javono]

==================================================
==================================================
Don Wood wrote
in this thread
https://coderanch.com/t/185337/java-developer-SCJD/certification/Lock
==================================================
Hi Javini,
Whenever you are dealing with nested locks, you must always get them in the same
order. Apparently, know this as you said the following:

Dead-locks are avoided by following the locking order rules: always
lock LockManager, then lock MicroData.

Since you are using nested locks correctly (always in the same order) you are
using one of the classic deadlock avoidance mechanisms. You still have several
issues to be evaluated:
* Is the flow of code obvious enough that a junior programmer will not miss the
significance of lock ordering? If I understand the code right, you have nested
locks that are acquired in different methods. This is much harder for anyone
(much less a junior programmer) to pick up on than if both synchronized blocks
were in one method.
* What happens to the concurrency when the inner lock is not available? The
thread goes to sleep still owning the outer lock. The sleeping thread can block
other threads from doing any useful work and things come to a screeching halt
until the locks are cleared.
* When you say in step 1, Gets a lock on the LockManager., does this mean a
lock on the LockManager object? If so, when the inner lock is busy, then the
thread goes to sleep owning the LockManager lock. I don't see how the thread
that wants to release the inner lock will be able to do so since the LockManager
is locked and the owner is asleep. This can't be right so I must not be
understanding what the LockManager lock is.
Perhaps its just my reading of this that is faulty but when you say

Usually, of course, clients are
operating outside of MicroData, first using the lock manager, then calling
Data.someMethod() which in turn calls the appropriate methods in MicroData.

Do you mean that the clients have the lock manager locked when they call
MicroData? If so, it seems that you can be holding the lock manager lock for
duration of doing I/O. That would seem like the system is overlocking and
concurrency is reduced.
The alternative interpretation that I see is that the clients use the lock
manager and release its lock before calling Data.someMethod() which in turn
calls the appropriate methods in MicroData. But since MicroData can then call
the LockManager that would be a violation of your contract that says the locks
are always locked in the same order.
Nested locks can be used in a deadlock safe manner. However, when you do use
them you have to answer a lot of questions. Does it truly avoid deadlock? What
are the performance implications, especially when the thread goes to sleep
holding the outer lock while waiting for the inner lock to become free? How hard
is it for someone else maintaing the code to understand the nested locks?
My understanding of the avoid nested locks rule, is that it can be done but it
will probably save you time and a lot of headaches to redesign the code to avoid
nested locks.
==================================================
==================================================
==================================================
==================================================
Hi,
Thanks for taking the time to read my post and give your response.
Before continuing, here are some lock-within-lock postings that already exist:
Topic: Does nest locks really bad?
https://coderanch.com/t/183186/java-developer-SCJD/certification/Does-nest-locks-really-bad
Topic: Order of releasing locks
https://coderanch.com/t/185309/java-developer-SCJD/certification/Order-releasing-locks
I'll read the above links and my software, and then get back with my reply.
Thanks again,
Javini Javono
[ March 27, 2004: Message edited by: Javini Javono ]

[Javini Javono]

Hi,
Here is a rough overview with more specifics.
The lock manager is called Guardian.
-----------------------------------
* The Guardian methods are rarely synchronized as it is multi-threaded.
However, when it does work, it locks the guard object which keeps track of
which records are currently locked. There are other structures, such as to
keep track of deleted record numbers. All these structures are used together
and are synchronized on guard whenever any work is done.
* Some Guardian methods call into DataCore class methods, such as to see if a
record number exists within the file.
Thus, the locking order is always the following:
lock guard, then call a method in DataCore which locks on dataCore instance.
dataCore instance
-----------------
The reader and writer with all its methods synchronized is called
DataCore (except that the create() methods are the only methods which are not
synchronized).
The create() method will use the lock manager which in turn will call back into
a synchronized method of dataCore, which, if not handled correctly, will result
in dead-lock.
The create() method needs to lock a record, whether a previously deleted record
or a record at the end of the file, prior to writing new information to it and
effectively creating this record.

    //This method used to be synchronized.But, it results in deadlock because
    //it also calls Guardian (which synchronizes on guard, and which in turn
    //calls back in to this class's synchronized methods).
    //
    //So, when Guardian methods are called, don't be synchronized on this
    //class's object, i.e. on this dataCore instance.
    
    int create(GrossFields grossFields,
               String clientId,
               long requestedPhysicalRecordNumber)
                                     throws DuplicateKeyException, IOException {

        //This is very important!  The rule to avoid dead lock is that you
        //lock the two objects in the correct order.  Thus, Guardian.guard
        //is always locked first, and then the dataCore instance.
        //If you take out the synchronization statement on guard, dead-lock will
        //occur.
        
        synchronized (Guardian.guard) {
            synchronized (this) {
                return createContinued(grossFields,
                                       clientId,
                                       requestedPhysicalRecordNumber);
            }
        }
    }
    
    //Do not synchronized this method or parts thereof at this time;  for
    //it is synchronized by two controls from method create() above.
    
    private int createContinued(GrossFields grossFields,
                                String clientId,
                                long requestedPhysicalRecordNumber)
                                     throws DuplicateKeyException, IOException {
    }

So, when a record is created, and the dataCore.create() method is called,
it first locks down all data structures associated with the lock manager.
Therefore, no new threads can query the lock manager nor request that any
record be locked.
Eventually, any threads that are reading and writing using any of dataCore's
synchronized methods will finish, and the create() method will get the lock
on "this" (which is the instance of dataCore).
The lock manager, Guardian, can then be safely used, including its abilities to
call back into synchronized methods of dataCore.
To answer your question: what happens to concurrency if the inner lock is not
available? I'd say not much, if the lock is not available, the dataCore object
is already busy reading, writing, or both.
The dataCore methods are not yet optimized. I have my JUnit code working, so
that subsequent optimization to heighten concurrency can occur easily in the
near future. So, in the future, dataCore's complete methods will not be
synchronized, only the critical sections within them will be synchronized on
"this". The same applies to the createContinued() method, its critical section
will be synchronized inside the code, using the the same, dual locks:
guard and this.
Another related question I have is what other design choice exists for creating
a record?
One idea I thought of while writing this out this morning, is that I could
create a new thread to carry out the create operation; this thread would be
sent to use a different object, which then, in turn, would use the lock manager
and then create a new record; then this thread would have to use a notify
pattern to tell the original thread the results of the operation. But, I think
my design as written is simpler to understand, and I think it does not harm
concurrency. (but, of course, if I was certain, then I probably would not be
posting this type of question).
Thanks,
Javini Javono

[Don Wood]

Looks like I invented a new term above, dealock safe should have been thread safe.
Javini said,

To answer your question: what happens to concurrency if the inner lock is not available? I'd say not much, if the lock is not available, the dataCore object is already busy reading, writing, or both.

When the inner lock is not available, this thread sleeps holding the guard lock. Isn't true that any other thread that wants the guard lock must now wait even if the work it is doing for a different record? Lock and unlcok should be fast operations but with nested locks the guard lock risks becoming a bottleneck. This is what I mean by reduced concurrency since threads doing unrelated work on other records must now wait for this thread which is asleep with the guard lock.