commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ozeigerm...@apache.org
Subject cvs commit: jakarta-commons/transaction/src/java/org/apache/commons/transaction/locking GenericLockManager.java
Date Tue, 14 Dec 2004 15:23:10 GMT
ozeigermann    2004/12/14 07:23:10

  Modified:    transaction/src/java/org/apache/commons/transaction/locking
                        GenericLockManager.java
  Log:
  Fixed race condition between deadlock check and actually acquiring the lock
  
  Revision  Changes    Path
  1.3       +33 -13    jakarta-commons/transaction/src/java/org/apache/commons/transaction/locking/GenericLockManager.java
  
  Index: GenericLockManager.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/transaction/src/java/org/apache/commons/transaction/locking/GenericLockManager.java,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- GenericLockManager.java	14 Dec 2004 12:12:46 -0000	1.2
  +++ GenericLockManager.java	14 Dec 2004 15:23:10 -0000	1.3
  @@ -111,17 +111,37 @@
   
           GenericLock lock = (GenericLock) atomicGetOrCreateLock(resourceId);
   
  -        boolean deadlock = wouldDeadlock(ownerId, lock, targetLockLevel,
  -                reentrant ? GenericLock.COMPATIBILITY_REENTRANT : GenericLock.COMPATIBILITY_NONE);
  -        if (deadlock) {
  -            throw new LockException("Lock would cause deadlock",
  -                    LockException.CODE_DEADLOCK_VICTIM, resourceId);
  -        }
  -
  +        // we need to be careful that we the detected deadlock status is still valid when
actually
  +        // applying for the lock
  +        // we have to take care that 
  +        // (a) no one else acquires the lock after we have done deadlock checking as this
would
  +        //    invalidate our checking result
  +        // (b) other threads that might concurrently apply for locks we are holding need
to know
  +        //    we are applying for this special lock before we check for deadlocks ourselves;
this
  +        //    is important as the other thread might be the one to discover the deadlock
  +        
  +        // (b) register us as a waiter before actually trying, so other threads take us
into account
           addWaiter(lock, ownerId);
  +
           try {
  -            boolean acquired = lock
  -                    .acquire(ownerId, targetLockLevel, true, reentrant, timeoutMSecs);
  +            boolean acquired;
  +            // (a) while we are checking if we can have this lock, no one else must apply
for it
  +            // and possibly change the data
  +            synchronized (lock) {
  +                
  +                // TODO: detection is rather expensive, would be an idea to wait for a

  +                // short time (<5 seconds) to see if we get the lock, after that we
can still check
  +                // for the deadlock and if not try with the remaining timeout time
  +                boolean deadlock = wouldDeadlock(ownerId, lock, targetLockLevel,
  +                        reentrant ? GenericLock.COMPATIBILITY_REENTRANT : GenericLock.COMPATIBILITY_NONE);
  +                if (deadlock) {
  +                    throw new LockException("Lock would cause deadlock",
  +                            LockException.CODE_DEADLOCK_VICTIM, resourceId);
  +                }
  +        
  +                acquired = lock
  +                        .acquire(ownerId, targetLockLevel, true, reentrant, timeoutMSecs);
  +            }
               if (!acquired) {
                   throw new LockException("Lock wait timed out", LockException.CODE_TIMED_OUT,
                           resourceId);
  
  
  

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message