DeadLock

[John Winstanley]

Hello All,

I'm looking at ways to prevent deadlock, this is my lock method. Do you think it is ok to throw an unchecked exception if it takes the client too long to obtain a lock.

  
private Map locks;    
public long lock(int recNo) throws RecordNotFoundException {            
    // get a cookie based on the time        
    synchronized (locks){                    
        while(locks.containsKey(recNo)) {                            
            locks.wait(10 seconds);             
            if(cookie is older than 10 seconds) {                
                // notify all threads waiting on the locks object
                throw new SomeLockError("Deadlock.");
            }        
        }                    
        // Check record exists                    
        // Check record is not deleted                    
        // put the record number and the cookie in the map
        // notify all threads waiting on the locks object            
    }             
    // return the cookie    
}

I think it is ok to do this becuase I state that anyone using my database API should hold a lock for the minimum amount of time possible. And in the cercumstance where a client is holding a lock for too long other clients will just have to retry later.

[Dieskun Koper]

My instructions say the following about the lock method:

'the current thread gives up the CPU and consumes no CPU cycles until the record is unlocked'

Using your method, the current thread gives up the CPU and consumes no CPU cycles until 10 seconds have passed or the record is unlocked.
I would not risk auto-failure here, especially as there are other ways to deal with deadlocks.

And I think it is bad design to throw an unchecked exception for this. Please read up on the difference between runtime and checked exceptions to understand why.

[Andrew Monkhouse]

Hi John,

I suspect that your solution will not meet the requirements. Check the wording of the API documentation you were provided for the lock method - does it imply that you can have a timeout at all? If you can have a timeout, do you think having the timeout on the thread attempting to aquire the lock is correct?

You also try to aquire the lock before checking whether the record is valid or not. You might want to consider having a separate (private) method to check validity of the record, in which case your code could be reduced to:

    while (recordIsValid(recNo) && locks.containsKey(recNo)) {
        locks.wait();
    }

The recordIsValid() method can throw the RecordNotFoundException directly.

Regards, Andrew

[peter wooster]

Originally posted by Andrew Monkhouse:

You also try to aquire the lock before checking whether the record is valid or not.

Andrew,

can you explain why checking for record validity before locking is of any benefit. I view the resource being locked as a simple numeric token, my LockManager doesn't know about the I/O at all. I do this to avoid nested synchronization problems. After the Data instance aquires the lock on the token, it checks if the token corresponds to a valid record and unlocks if it doesn't.

Any checking done before aquiring the lock must be redone after aquiring it. This checking might save an occasional lock attempt, but would add I/O overhead.

[Andrew Monkhouse]

Hi Peter,

In retrospect I am sorry I suggested this. With a cached database my solution might catch an attempt to lock an invalid record slightly quicker than your / John's solution - but only slightly quicker.

Please ignore my earlier suggestion.

Regards, Andrew

[GD Deepz]

I am using the leasing solution deadlocks or orphan locks.
Have a dameon thread running in the background to check if a locked record has been locked for certain time limit. Call unlock and notifyAll() if the lease has expired for the locked record.

However, the client is never informed by the server when the locked record has been unlocked. Not taking this into consideration.

Any feedback?

[Reza Rahman]

Although it might not hurt anything, I would avoid using a deamon thread if possible. Could you not check for an orphaned lock in the lock method itself (e.g. in the isValid() method mentioned above)?