wood burning stoves 2.0*
The moose likes Developer Certification (SCJD/OCMJD) and the fly likes NX: Nested synchronized blocks Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Certification » Developer Certification (SCJD/OCMJD)
Bookmark "NX: Nested synchronized blocks" Watch "NX: Nested synchronized blocks" New topic
Author

NX: Nested synchronized blocks

Aleh Barysevich
Greenhorn

Joined: Dec 03, 2003
Posts: 6
Hello everyone,
It has been discussed a lot that nested synchronization blocks are quite dangerous and may leed to deadlocks.
But let's take a look at:
public synchronized long lock(int recNo) throws RecordNotFoundException;
As far as I understand many of those who had this method with the same signature used synchronization on the object which contains locked records (smth like

where lockedRecords is some sort of collection/map/...).
At the same checkRecordValidity(recNo) is the method that must be synchronized itself on some other object (e.g. RAF-object if you do not implement any cache and read directly from file).
So we have 2 nested synchronized block here.
I feel like I missed something but can't get where did I go wrong. Is it ok to have nested synchronized blocks or maybe there are ways to get rid of them in this situation. Please help.
Tank you,
Aleh
[ January 12, 2004: Message edited by: Aleh Barysevich ]
George Marinkovich
Ranch Hand

Joined: Apr 15, 2003
Posts: 619
Hi Aleh,
Originally posted by Aleh Barysevich:


Hmmmm, lock (as written) wouldn't meet my criterion for making a Data method synchronized as it doesn't access the database file as near as I can tell. I suggest lock can be unsynchronized because the only shared (non local variable) that it accesses is lockedRecords and when it does access lockedRecords it does so in a synchronized (lockedRecords) block.
Hope this helps,
George


Regards, George
SCJP, SCJD, SCWCD, SCBCD
Aleh Barysevich
Greenhorn

Joined: Dec 03, 2003
Posts: 6
Hi George,
I'm terribly sorry for the confusion - surely the method itself mustn't be synchronized.
I've edited the original message - removed that mistaken 'synchronized'.
Thank you,
Aleh
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11508
    
  95

Hi Aleh,
Originally posted by Aleh Barysevich:
At the same checkRecordValidity(recNo) is the method that must be synchronized itself on some other object (e.g. RAF-object if you do not implement any cache and read directly from file).

Why do you have to synchronize it on some other object?
Originally posted by Aleh Barysevich:
Is it ok to have nested synchronized blocks or maybe there are ways to get rid of them in this situation. Please help.

Having nested locking can lead to deadlock. Same as having a car can lead to having an accident. But just because you have a car does not mean that you will have an accident. And having nested locking does not mean that you will have a deadlock.
My recommendation is to write down all the locks that you have. Then write down which locking methods can be called while you own a lock. For example:
  • Own lock on A. Can try to get lock on B
  • Own lock on B. Can try to get locks on C or E
  • Own lock on C. Can try to get lock on D
  • Own lock on D. Cannot get any other locks
  • Own lock on E. Cannot get any other locks


  • Woohoo. There is no chance for deadlock in that situation - at no point do I try to get an earlier lock. If you prefer it in tree form:

    I can only ever move down the tree. A can only ever get to B. B can never get to A. So I am safe.
    Contrast this with:
  • Own lock on A. Can try to get lock on B
  • Own lock on B. Can try to get locks on C or E
  • Own lock on C. Can try to get lock on D or A
  • Own lock on D. Cannot get any other locks
  • Own lock on E. Cannot get any other locks


  • Oops: A can (eventually) get a lock on C. And someone who has a lock on C can try to get a lock on A. So there is the potential for deadlock.
    In tree form again:
    I can move up the tree so I am in trouble!
    To refer back to your example: while you own the mutex on lockedRecords, you are trying to get some other mutex for validation (I am going to call it validationMutex). Now while you own the mutex on validationMutex, will you ever try to get any other mutex's? If not, you should be safe.
    By the way, I would strongly recommend that everyone go through the logical exercise of determining whether there are any conditions that logically might lead to deadlock (much more important that trying to "prove it" using coded tests). You may like to write it down in either the point form or the tree form that I have done above. Then when you have determined that logically you cannot deadlock, then add that proof into your documentation. It may save the examiner a lot of time if they can see that you have already done the work for them: they will just need to validate your proof .
    Regards, Andrew
    [ January 13, 2004: Message edited by: Andrew Monkhouse ]

    The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
    Aleh Barysevich
    Greenhorn

    Joined: Dec 03, 2003
    Posts: 6
    Hi Andrew,
    Thank you for such a detailed answer.

    Why do you have to synchronize it on some other object?

    The method checkRecordValidity(recNo); should check whether the record is not deleted, recNo is in proper bounds, maybe some other things...
    So if you do not implement any cache and read directly from file, you have to synchronize on RAF-object to perform seek() and read() as one atomic operation. Makes sence?

    Then when you have determined that logically you cannot deadlock, then add that proof into your documentation.

    Frankly speaking this was the main reason for my question: is it ok just to add a proof in my choices.txt? Won't be I penalize´┐Ż just for having such a coding construction in my code?
    Thank you,
    Aleh
    Andrew Monkhouse
    author and jackaroo
    Marshal Commander

    Joined: Mar 28, 2003
    Posts: 11508
        
      95

    Hi Aleh,
    Originally posted by Aleh Barysevich:
    Frankly speaking this was the main reason for my question: is it ok just to add a proof in my choices.txt? Won't be I penalize´┐Ż just for having such a coding construction in my code?

    It comes down to a design decision (and therefore should be documented ):
  • if you only have one mutex for all synchronized blocks, then there is no chance of deadlock. So you won't need to document anything.
  • however if you have different mutexes depending on what the synchronized block is doing, then you will have better performance. But then you should prove logically that you cannot deadlock.


  • You should not get penalized for having multiple mutexes, as long as you have documented why you are using them, and have considered the deadlock issues.
    Regards, Andrew
    Terry Martinson
    Ranch Hand

    Joined: Oct 18, 2003
    Posts: 293
    Andrew -

    By the way, I would strongly recommend that everyone go through the logical exercise of determining whether there are any conditions that logically might lead to deadlock (much more important that trying to "prove it" using coded tests). You may like to write it down in either the point form or the tree form that I have done above. Then when you have determined that logically you cannot deadlock, then add that proof into your documentation. It may save the examiner a lot of time if they can see that you have already done the work for them: they will just need to validate your proof <http://www.javaranch.com .

    Thanks for the suggestion. An excellent idea.
    TJ


    SCJP, SCJD, SCWCD, SCBCD
     
    Don't get me started about those stupid light bulbs.
     
    subject: NX: Nested synchronized blocks