[jboss-cvs] JBossCache/src/org/jboss/cache ...

[Brian Stansberry]

  User: bstansberry
  Date: 06/12/07 20:26:15

  Modified:    src/org/jboss/cache  Tag: Branch_JBossCache_1_4_0
                        TransactionEntry.java
  Log:
  [JBCACHE-874] Store locks in a LinkedHashSet
  
  Revision  Changes    Path
  No                   revision
  
  
  No                   revision
  
  
  1.8.2.2   +22 -23    JBossCache/src/org/jboss/cache/TransactionEntry.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: TransactionEntry.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossCache/src/org/jboss/cache/TransactionEntry.java,v
  retrieving revision 1.8.2.1
  retrieving revision 1.8.2.2
  diff -u -b -r1.8.2.1 -r1.8.2.2
  --- TransactionEntry.java	5 Dec 2006 00:01:51 -0000	1.8.2.1
  +++ TransactionEntry.java	8 Dec 2006 01:26:15 -0000	1.8.2.2
  @@ -17,6 +17,7 @@
   import java.util.ArrayList;
   import java.util.Collection;
   import java.util.Iterator;
  +import java.util.LinkedHashSet;
   import java.util.LinkedList;
   import java.util.List;
   import java.util.ListIterator;
  @@ -35,7 +36,7 @@
    * </ul>
    *
    * @author <a href="mailto:bela at jboss.org">Bela Ban</a> Apr 14, 2003
  - * @version $Revision: 1.8.2.1 $
  + * @version $Revision: 1.8.2.2 $
    */
   public class TransactionEntry {
   
  @@ -62,9 +63,12 @@
      protected List undo_list=new LinkedList();
   
      /**
  -    * List<IdentityLock> of locks acquired by the transaction ({@link IdentityLock})
  +    * LinkedHashSet<IdentityLock> of locks acquired by the transaction We use
  +    * a LinkedHashSet because we need efficient Set semantics (same lock can
  +    * be added multiple times) but also need guaranteed ordering for use
  +    * by lock release code (see JBCCACHE-874).
       */
  -   protected List locks = new LinkedList();
  +   private LinkedHashSet locks = new LinkedHashSet();
   
      /**
       * A list of dummy uninitialised nodes created by the cache loader interceptor to load data for a
  @@ -163,13 +167,10 @@
      public void addLock(IdentityLock l) {
         if(l != null) {
            synchronized(locks) {
  -             if (!locks.contains(l))
  -             {
                    locks.add(l);
                }
            }
         }
  -   }
   
      /**
       * Add multiple locks to the lock list.
  @@ -177,25 +178,22 @@
       */
      public void addLocks(Collection newLocks) {
         if(newLocks != null) {
  -         IdentityLock tmp;
            synchronized(locks) {
  -            for(Iterator it=newLocks.iterator(); it.hasNext();) {
  -               tmp=(IdentityLock)it.next();
  -                if (!locks.contains(tmp))
  -                {
  -                    locks.add(tmp);
  -                }
  -            }
  +            locks.addAll(newLocks);
            }
         }
      }
   
      /**
       * Returns the locks in use.
  -    * Note:  This list may be concurrently modified.
  +    * 
  +    * @return a defensive copy of the internal data structure.
       */
      public List getLocks() {
  -      return locks;
  +      synchronized(locks)
  +      {
  +         return new ArrayList(locks);
  +      }
      }
   
      /**
  @@ -214,16 +212,17 @@
       * Clears the list of locks held.
       */
      public void releaseAllLocksLIFO(Object owner) {
  -      // I guess a copy would work as well
  -      // This seems fairly safe though
  +      
         synchronized (locks) {
  -         for (ListIterator i = locks.listIterator(locks.size()); i.hasPrevious();) {
  -            IdentityLock lock = (IdentityLock)i.previous();
  +         // Copying out to an array is faster than creating an ArrayList and iterating,
  +         // since list creation will just copy out to an array internally
  +         IdentityLock[] lockArray = (IdentityLock[]) locks.toArray(new IdentityLock[locks.size()]);
  +         for (int i = lockArray.length -1; i >= 0; i--) {
                if (log.isTraceEnabled())
                {
  -                 log.trace("releasing lock for " + lock.getFqn() + " (" + lock + ")");
  +                 log.trace("releasing lock for " + lockArray[i].getFqn() + " (" + lockArray[i] + ")");
                }
  -             lock.release(owner);
  +             lockArray[i].release(owner);
            }
            locks.clear();
         }