You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@river.apache.org by pe...@apache.org on 2013/06/04 00:01:38 UTC

svn commit: r1489197 - in /river/jtsk/skunk/qa_refactor/trunk: qa/src/com/sun/jini/test/impl/outrigger/leasing/ src/com/sun/jini/outrigger/

Author: peter_firmstone
Date: Mon Jun  3 22:01:38 2013
New Revision: 1489197

URL: http://svn.apache.org/r1489197
Log:
Fix failing Outrigger test com/sun/jini/test/impl/outrigger/leasing/UseNotifyLeaseTest, caused by mutation of arrays in EntryRep after publication, see safe publication comments in EntryRep.  Clean up and simplification of Outrigger code.

Removed:
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHandleHashDesc.java
Modified:
    river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/impl/outrigger/leasing/LeasedSpaceListener.java
    river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/impl/outrigger/leasing/UseNotifyLeaseTest.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/BaseHandle.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ConsumingWatcher.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHandle.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHolder.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHolderSet.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryRep.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/OperationJournal.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/OutriggerServerImpl.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ReadIfExistsWatcher.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TakeIfExistsWatcher.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TakeMultipleWatcher.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TemplateHandle.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransactableReadIfExistsWatcher.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransitionWatcher.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransitionWatchers.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/Txn.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnMonitor.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnMonitorTask.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnState.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TypeTree.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/WatchersForTemplateClass.java

Modified: river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/impl/outrigger/leasing/LeasedSpaceListener.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/impl/outrigger/leasing/LeasedSpaceListener.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/impl/outrigger/leasing/LeasedSpaceListener.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/impl/outrigger/leasing/LeasedSpaceListener.java Mon Jun  3 22:01:38 2013
@@ -113,7 +113,7 @@ public class LeasedSpaceListener
             received = true;
             this.notifyAll();
         }
-        logger.log(Level.INFO, "notify called at {0}", date.getTime());
+        logger.log(Level.FINER, "notify called at {0}", date.getTime());
     }
 
     /**

Modified: river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/impl/outrigger/leasing/UseNotifyLeaseTest.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/impl/outrigger/leasing/UseNotifyLeaseTest.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/impl/outrigger/leasing/UseNotifyLeaseTest.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/impl/outrigger/leasing/UseNotifyLeaseTest.java Mon Jun  3 22:01:38 2013
@@ -103,10 +103,6 @@ public class UseNotifyLeaseTest extends 
             synchronized (this){
                 logger.log(Level.FINEST, "Writing entry {0}", ++count);
                 synchronized (listener) {
-                    // This doesn't look atomic, if the event is sent too
-                    // quickly, this will just set it false.  Alternative;
-                    // reset listener immediately after we receive it.
-//                    listener.setReceived(false);
                    assert listener.isReceived() == false;
 
                     /*
@@ -123,7 +119,7 @@ public class UseNotifyLeaseTest extends 
                         listener.wait(callbackWait);
                         if (listener.isReceived()){
                             logger.log(Level.FINEST, "Wait done at {0}, received = {1}", new Object[]{new java.util.Date(), listener.isReceived()});
-                            // Reset listener, see comment above.
+                            // Reset listener state.
                             listener.setReceived(false);
                             return true;
                         }

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/BaseHandle.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/BaseHandle.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/BaseHandle.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/BaseHandle.java Mon Jun  3 22:01:38 2013
@@ -17,21 +17,29 @@
  */
 package com.sun.jini.outrigger;
 
+import java.util.Queue;
+
 /**
  * Base class for handles to Entries and templates.
  *
  * @author Sun Microsystems, Inc.
  *
  */
-class BaseHandle  {
+abstract class BaseHandle  {
     private final EntryRep rep;		// the rep this handle manages
-    private boolean removed = false;
+    private final Queue<? extends BaseHandle> content;
 
     /**
      * Create a new handle
+     * 
+     * @param content thread safe Queue from which this BaseHandle will be removed
+     * atomically, BaseHandle is not added to content during construction,
+     * as it would allow this to escape.
+     * @param rep EntryRep managed by this BaseHandle.
      */
-    BaseHandle(EntryRep rep) {
+    protected BaseHandle(EntryRep rep, Queue<? extends BaseHandle> content) {
 	this.rep = rep;
+        this.content = content;
     }
 
     /**
@@ -46,18 +54,15 @@ class BaseHandle  {
 	return rep.classFor();
     }
     
-    public synchronized boolean removed(){
-        return removed;
-    }
+    public abstract boolean removed();
     
     /**
+     * Overridden and called from subclass.
      * 
-     * @return true if this is the first time remove is called.
+     * @return true if removed.
      */
-    public synchronized boolean remove(){
-        boolean result = !removed;
-        removed = true;
-        return result;
+    public boolean remove(){
+        return content.remove(this);
     }
 }
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ConsumingWatcher.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ConsumingWatcher.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ConsumingWatcher.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ConsumingWatcher.java Mon Jun  3 22:01:38 2013
@@ -18,6 +18,7 @@
 package com.sun.jini.outrigger;
 
 import java.util.Map;
+import java.util.Set;
 import java.util.WeakHashMap;
 import net.jini.core.transaction.TransactionException;
 import net.jini.space.InternalSpaceException;
@@ -47,7 +48,7 @@ class ConsumingWatcher extends Singleton
      * we would have liked to return, but have been provisionally
      * removed.
      */
-    private final Map provisionallyRemovedEntrySet;
+    private final Set<EntryHandle> provisionallyRemovedEntrySet;
 
     /**
      * Create a new <code>ConsumingWatcher</code>.
@@ -74,7 +75,7 @@ class ConsumingWatcher extends Singleton
      *        <code>false</code> otherwise.  
      */
     ConsumingWatcher(long expiration, long timestamp, long startOrdinal, 
-		     Map provisionallyRemovedEntrySet, Txn txn,
+		     Set<EntryHandle> provisionallyRemovedEntrySet, Txn txn,
 		     boolean takeIt)
     {
 	super(expiration, timestamp, startOrdinal);

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHandle.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHandle.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHandle.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHandle.java Mon Jun  3 22:01:38 2013
@@ -19,6 +19,7 @@ package com.sun.jini.outrigger;
 
 import net.jini.io.MarshalledInstance;
 import com.sun.jini.landlord.LeasedResource;
+import java.util.Queue;
 
 /**
  * This object holds an annotated reference to an
@@ -123,6 +124,7 @@ class EntryHandle extends BaseHandle imp
      * but the removal has not yet been committed to disk
      */
     private boolean removePending = false;
+    private boolean removed = false;
 
     /**
      * Create a new handle, calculating the hash for the object.
@@ -134,9 +136,10 @@ class EntryHandle extends BaseHandle imp
      * @param holder If mgr is non-<code>null</code> this must be
      *            the holder holding this handle.  Otherwise it may be
      *            <code>null</code> 
+     * @param content Queue this EntryHandle will be removed from.
      */
-    EntryHandle(EntryRep rep, TransactableMgr mgr, EntryHolder holder) {
-	super(rep);
+    EntryHandle(EntryRep rep, TransactableMgr mgr, EntryHolder holder, Queue<EntryHandle> content) {
+	super(rep, content);
 	hash = (rep != null ? hashFor(rep, rep.numFields())[0] : -1);
 	if (mgr == null) {
 	    txnState = null;
@@ -161,16 +164,6 @@ class EntryHandle extends BaseHandle imp
     }
 
     /**
-     * Calculate the hash for a particular entry, assuming the given number
-     * of fields.
-     *
-     * @see #hashFor(EntryRep,int,EntryHandleHashDesc)
-     */
-//    static long hashFor(EntryRep rep, int numFields) {
-//	return hashFor(rep, numFields, null);
-//    }
-
-    /**
      * Calculate the hash for a particular entry, assuming the given
      * number of fields, filling in the fields of <code>desc</code>
      * with the relevant values.  <code>desc</code> may be
@@ -219,7 +212,6 @@ class EntryHandle extends BaseHandle imp
      * @see EntryHandleTmplDesc
      */
     static EntryHandleTmplDesc descFor(EntryRep tmpl, int numFields) {
-//	EntryHandleHashDesc hashDesc = new EntryHandleHashDesc();
 	EntryHandleTmplDesc tmplDesc;
         
 	// Get the hash and the related useful information
@@ -276,7 +268,7 @@ class EntryHandle extends BaseHandle imp
     // EntryHolder.SimpleRepEnum.nextRep().  Working it through
     // it seems to work in that particular case, but it seems fragile.
     boolean canPerform(TransactableMgr mgr, int op) {
-        synchronized (this){ // Audit revealed calling thread didn't always own lock.
+        synchronized (this){ // Audit revealed calling thread didn't always own lock see comment above.
             if (txnState == null) 
                 return true; // all operations are legal on a non-transacted entry
 
@@ -452,4 +444,14 @@ class EntryHandle extends BaseHandle imp
 	if (last)
 	    txnState = null;
     }
+
+    @Override
+    public synchronized boolean removed() {
+        return removed;
+    }
+    
+    public synchronized boolean remove() {
+        if (!removed) removed = super.remove();
+        return removed;
+    }
 }

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHolder.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHolder.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHolder.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHolder.java Mon Jun  3 22:01:38 2013
@@ -24,6 +24,7 @@ import java.util.Queue;
 import java.util.Set;
 import java.util.WeakHashMap;
 import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.ConcurrentMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -52,7 +53,7 @@ class EntryHolder implements Transaction
      * <code>EntryHolderSet</code> and every other
      * <code>EntryHolder</code>.  
      */
-    private final Map<Uuid, EntryHandle> idMap;
+    private final ConcurrentMap<Uuid, EntryHandle> idMap;
 
     /** The server we are working for */
     private final OutriggerServerImpl space;
@@ -72,10 +73,14 @@ class EntryHolder implements Transaction
      * <code>EntryHolderSet</code> so that there is one table that can
      * map ID to <code>EntryRep</code>
      */
-    EntryHolder(OutriggerServerImpl space, Map<Uuid,EntryHandle> idMap) {
+    EntryHolder(OutriggerServerImpl space, ConcurrentMap<Uuid,EntryHandle> idMap) {
 	this.space = space;
 	this.idMap = idMap;
     }
+    
+    EntryHandle newEntryHandle(EntryRep rep, TransactableMgr mgr){
+        return new EntryHandle(rep, mgr, this, content);
+    }
 
     /**
      * Return an <code>EntryHandle</code> object that matches the given
@@ -122,7 +127,7 @@ class EntryHolder implements Transaction
      */
     EntryHandle hasMatch(EntryRep tmpl, TransactableMgr txn, boolean takeIt,
             Set conflictSet, Set lockedEntrySet,
-            Map provisionallyRemovedEntrySet)
+            Set provisionallyRemovedEntrySet)
             throws CannotJoinException {
         matchingLogger.entering("EntryHolder", "hasMatch");
         EntryHandleTmplDesc desc = null;
@@ -219,7 +224,7 @@ class EntryHolder implements Transaction
      */
     boolean attemptCapture(EntryHandle handle, TransactableMgr txn,
 	boolean takeIt, Set conflictSet, Set lockedEntrySet, 
-        Map provisionallyRemovedEntrySet, long now)
+        Set provisionallyRemovedEntrySet, long now)
     {
 	try {
 	    return confirmAvailabilityWithTxn(handle.rep(), handle,
@@ -236,8 +241,8 @@ class EntryHolder implements Transaction
      */
     private boolean confirmAvailabilityWithTxn(EntryRep rep, 
 	      EntryHandle handle, TransactableMgr txnMgr, boolean takeIt, 
-	      long time, Set conflictSet, Set lockedEntrySet,
-	      Map provisionallyRemovedEntrySet)
+	      long time, Set conflictSet, Set<Uuid> lockedEntrySet,
+	      Set<EntryHandle> provisionallyRemovedEntrySet)
 	throws CannotJoinException
     {
 	// Now that we know we have a match, make sure that the the
@@ -280,8 +285,8 @@ class EntryHolder implements Transaction
     private boolean
 	confirmAvailability(EntryRep rep, EntryHandle handle,
 	      TransactableMgr txn, boolean takeIt, long time,
-	      Set conflictSet, Set lockedEntrySet,
-	      Map provisionallyRemovedEntrySet)
+	      Set conflictSet, Set<Uuid> lockedEntrySet,
+	      Set<EntryHandle> provisionallyRemovedEntrySet)
     {
 	synchronized (handle) {
             if (handle.removed()) return false;
@@ -291,7 +296,7 @@ class EntryHolder implements Transaction
 		
 	    if (handle.isProvisionallyRemoved()) {
 		if (provisionallyRemovedEntrySet != null)
-		    provisionallyRemovedEntrySet.put(handle, null);
+		    provisionallyRemovedEntrySet.add(handle);
 		return false;
 	    }
 
@@ -525,31 +530,19 @@ class EntryHolder implements Transaction
 	/* Make sure info duplicated across all the handles in this
 	 * holder is shared.
 	 *
-	 * Because this thread will usually hold the lock on handle,
-	 * we must call contents.head before calling contents.add
-	 * otherwise during the head call this thread will attempt to
-	 * obtain a lock on a 2nd FastList node in the same FastList,
-	 * and make that attempt in the wrong order. In particular
-	 * this could lead to deadlocks when SimpleRepEnum or
-	 * ContinuingQuery call into FastList.Node.restart ( restart
-	 * locks the node its called on (which can be the same node
-	 * head locks) and then tries to lock the tail (which can be
-	 * handle if add has already been called)). By calling head
-	 * before calling add we have locks on two FastList.Node
-	 * objects, but they are not in the same list so we are ok.
-	 *
 	 * We make the head call before calling txn.add because
 	 * it seems like better hygiene to fix reps state before
 	 * exposing it to others (even though shareWith will
 	 * not change handle's state materially).  
 	 */
 	final EntryHandle head = getContentsHead();
-	if (head != null && head != handle)
-	    rep.shareWith(head.rep());
-
-	if (txn != null) txn.add(handle);
-	content.add(handle);
-	idMap.put(rep.getCookie(), handle);
+	if (head != null && head != handle) rep.shareWith(head.rep());
+        synchronized (handle){ //typically synchronized externally anyway.
+            if (txn != null) txn.add(handle);
+            content.add(handle);
+            EntryHandle existed = idMap.putIfAbsent(rep.getCookie(), handle);
+            if (existed != null) throw new IllegalStateException("An EntryHandle with that Cookie already exists in idMap");
+        }
     }
 
     /**
@@ -654,7 +647,7 @@ class EntryHolder implements Transaction
 	final private boolean takeThem;
 
 	/** <code>EntryHandleTmplDesc</code> for the templates */
-	private volatile EntryHandleTmplDesc[] descs;
+        final private ThreadLocal<EntryHandleTmplDesc[]> descLocal;
 	    
 
 	/** Time used to weed out expired entries, ok if old */
@@ -689,6 +682,7 @@ class EntryHolder implements Transaction
 	    this.takeThem = takeThem;
 	    this.now = now;
 	    contentsIterator = content.iterator();
+            descLocal = new ThreadLocal<EntryHandleTmplDesc[]>();
 	}
 
 	/**
@@ -703,9 +697,7 @@ class EntryHolder implements Transaction
 	 *                 expired entries, ok if old
 	 */
 	void restart(long now) {
-	    if (!contentsIterator.hasNext())
-		return;
-
+	    if (!contentsIterator.hasNext()) return;
 	    this.now = now;
 	}
 
@@ -747,13 +739,11 @@ class EntryHolder implements Transaction
 	 *         the operation is to be performed under a transaction,
 	 *         but the transaction is no longer active.
 	 */
-	EntryHandle next(Set conflictSet, Set lockedEntrySet,
-			 Map provisionallyRemovedEntrySet) 
+	EntryHandle next(Set conflictSet, Set<Uuid> lockedEntrySet, Set<EntryHandle> provisionallyRemovedEntrySet) 
 	    throws CannotJoinException
 	{
 	    matchingLogger.entering("ContinuingQuery", "next");
-
-
+            EntryHandleTmplDesc[] descs = descLocal.get();
 	    while (contentsIterator.hasNext()) {
 	        EntryHandle handle = contentsIterator.next();
 	        if(descs == null){
@@ -763,17 +753,15 @@ class EntryHolder implements Transaction
 	                descs[i] = EntryHandle.descFor(tmpls[i], 
 	                                               handle.rep().numFields());
 	            }
+                    descLocal.set(descs);
 	        }
-		if (handleMatch(handle)) {
-		    
+		if (handleMatch(handle, descs)) {
 		    final boolean available =
 			confirmAvailabilityWithTxn(handle.rep(), handle, txn, 
 			    takeThem, now, conflictSet, lockedEntrySet, 
 			    provisionallyRemovedEntrySet);
 
-		    if (available) {
-			return handle;
-		    }
+		    if (available) return handle;
 		}
 	    }
 
@@ -784,7 +772,7 @@ class EntryHolder implements Transaction
 	 * Returns <code>true</code> if handle has not been removed
 	 * and matches one or more of the templates 
 	 */
-	private boolean handleMatch(EntryHandle handle) {
+	private boolean handleMatch(EntryHandle handle, EntryHandleTmplDesc[] descs) {
 	    if (handle.removed()) return false;
             int length = tmpls.length;
 	    for (int i=0; i<length; i++) {
@@ -815,59 +803,40 @@ class EntryHolder implements Transaction
     boolean remove(EntryHandle h, boolean recovery) {
 	boolean ok = false;
         synchronized (h){
-            ok =content.remove(h);
-            assert h.remove();
+            ok = h.remove();
+            if (!ok) throw new AssertionError("EntryHandle not removed");
             h.removalComplete();
+            // Ensure removal of EntryHandle is atomic.
+            boolean removed = idMap.remove(h.rep().getCookie(), h);
+            if (!removed) throw new IllegalStateException ("EntryHandle was missing from idMap at time of removal");
+            /* This may cause an ifExists query to be resolved,
+             * even if this entry was not locked under a transaction 
+             */
+            if (!recovery) {
+                space.recordTransition(
+                    new EntryTransition(h, null, false, false, false));
+            }
         }
-	if (ok) {
-	    idMap.remove(h.rep().getCookie());
-	    /* This may cause an ifExists query to be resolved,
-	     * even if this entry was not locked under a transaction 
-	     */
-	    if (!recovery) {
-		space.recordTransition(
-		    new EntryTransition(h, null, false, false, false));
-	    }
-	}
-
 	return ok;
     }
 
     /**
-     * Return the handle for the given <code>EntryRep</code> object.  
-     * This is done via lookup in the <code>idMap</code>.  The 
-     * <code>idMap</code> is doing double duty here.  The 
-     * <code>LeaseDesc</code> associated with an <code>EntryRep</code> 
-     * is also the rep's <code>EntryHandle</code>.
-     */
-    private EntryHandle handleFor(EntryRep rep) {
-	return idMap.get(rep.getCookie());
-    }
-
-    /**
-     * Reap the expired elements (and the underlying FastList)
+     * Reap the expired elements.
      */
     void reap() {
 
-	// Examine each of the elements within the FastList to determine if
+	// Examine each of the elements to determine if
 	// any of them have expired. If they have, ensure that they are
 	// removed ("reaped") from the collection. 
 
 	long now = System.currentTimeMillis();
 	for(EntryHandle handle : content){
 	    // Don't try to remove things twice
-//	    if (handle.removed()) {
-//		continue;
-//	    }
-
+	    if (handle.removed()) continue;
 	    // Calling isExpired() will both make the check and remove it
 	    // if necessary.
 	    isExpired(now, handle);
 	}
-
-	// This provides the FastList with an opportunity to actually
-	// excise the items identified as "removed" from the list.
-//	contents.reap();
     }
 }
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHolderSet.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHolderSet.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHolderSet.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryHolderSet.java Mon Jun  3 22:01:38 2013
@@ -24,6 +24,7 @@ import java.util.Collection;
 import net.jini.core.lease.Lease;
 import net.jini.id.Uuid;
 import com.sun.jini.landlord.LeasedResource;
+import java.util.Iterator;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
@@ -39,7 +40,7 @@ final class EntryHolderSet {
     private final ConcurrentMap<String,EntryHolder> holders = new ConcurrentHashMap<String,EntryHolder>();
     // Map of LeaseDescs indexed by the cookie of the underlying
     // LeasedResource.
-    private final Map<Uuid, EntryHandle> idMap = new Hashtable<Uuid, EntryHandle>();
+    private final ConcurrentMap<Uuid, EntryHandle> idMap = new ConcurrentHashMap<Uuid, EntryHandle>();
 
     private final OutriggerServerImpl space;
 
@@ -111,15 +112,19 @@ final class EntryHolderSet {
 	 * clone would probably work well since we don't add (and
 	 * never remove) elements that often.)
 	 */
-	final EntryHolder content[];
-	synchronized (holders) {
-	    final Collection values = holders.values();
-	    content = new EntryHolder[values.size()];
-	    values.toArray(content);
-	}
-
-	for (int i=0; i<content.length; i++) {
-            content[i].reap();
-	}	    
+//	final EntryHolder content[];
+//	synchronized (holders) {
+//	    final Collection values = holders.values();
+//	    content = new EntryHolder[values.size()];
+//	    values.toArray(content);
+//	}
+//
+//	for (int i=0; i<content.length; i++) {
+//            content[i].reap();
+//	}
+        Iterator<EntryHolder> it = holders.values().iterator();
+        while (it.hasNext()){
+            it.next().reap();
+        }
     }
 }

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryRep.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryRep.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryRep.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EntryRep.java Mon Jun  3 22:01:38 2013
@@ -101,7 +101,7 @@ class EntryRep implements StorableResour
     private static final EntryRep matchAnyRep;
 
     static {
-        classHashes = new WeakHashMap();
+        classHashes = new WeakHashMap<Class,Long>();
 	try {
 	    matchAnyRep = new EntryRep(new Entry() {
 		// keeps tests happy
@@ -163,7 +163,7 @@ class EntryRep implements StorableResour
      * Cached hash values for all classes we encounter. Weak hash used
      * in case the class is GC'ed from the client's VM.
      */
-    static final private WeakHashMap classHashes;
+    static final private WeakHashMap<Class,Long> classHashes;
 
     /**
      * Lookup the hash value for the given class. If it is not
@@ -175,7 +175,7 @@ class EntryRep implements StorableResour
 	throws MarshalException, UnusableEntryException
     {
 
-	Long hash = (Long)classHashes.get(clazz);
+	Long hash = classHashes.get(clazz);
 
 	// If hash not cached, calculate it for this class and,
 	// recursively, all superclasses
@@ -288,9 +288,10 @@ class EntryRep implements StorableResour
 	}
 
 	// copy the vals with the correct length
-	this.values = new MarshalledInstance[nvals];
-	System.arraycopy(vals, 0, this.values, 0, nvals);
-
+	MarshalledInstance [] values = new MarshalledInstance[nvals];
+	System.arraycopy(vals, 0, values, 0, nvals);
+        this.values = values; // safe publication
+        
 	try {
 	    hash = findHash(realClass, true).longValue();
 	} catch (UnusableEntryException e) {
@@ -299,8 +300,8 @@ class EntryRep implements StorableResour
 	}
 
 	// Loop through the supertypes, making a list of all superclasses.
-	ArrayList sclasses = new ArrayList();
-	ArrayList shashes = new ArrayList();
+	ArrayList<String> sclasses = new ArrayList<String>();
+	ArrayList<Long> shashes = new ArrayList<Long>();
 	for (Class c = realClass.getSuperclass();
 	     c != Object.class;
 	     c = c.getSuperclass())
@@ -315,11 +316,12 @@ class EntryRep implements StorableResour
 		throw new AssertionError(e);
 	    }
 	}
-	superclasses = (String[])sclasses.toArray(new String[sclasses.size()]);
-	hashes = new long[shashes.size()];
+	superclasses = sclasses.toArray(new String[sclasses.size()]); // safe publication.
+	long [] hashes = new long[shashes.size()];
 	for (int i=0; i < hashes.length; i++) {
-	    hashes[i] = ((Long)shashes.get(i)).longValue();
+	    hashes[i] = (shashes.get(i)).longValue();
 	}
+        this.hashes = hashes; // safe publication.
     }
 
     /**
@@ -891,7 +893,7 @@ class EntryRep implements StorableResour
 
     /**
      * Construct, log, and throw a new UnusableEntryException, that
-     * raps a given exception.
+     * wraps a given exception.
      */
     private static UnusableEntryException throwNewUnusableEntryException(
             Throwable nested) 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/OperationJournal.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/OperationJournal.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/OperationJournal.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/OperationJournal.java Mon Jun  3 22:01:38 2013
@@ -440,13 +440,13 @@ class OperationJournal extends Thread {
 			AssertionError("JournalNode with null payload");
 		} else if (payload instanceof EntryTransition) {
 		    final EntryTransition t = (EntryTransition)payload;
-		    final SortedSet set = 
+		    final SortedSet<TransitionWatcher> set = 
 			watchers.allMatches(t, ordinal);
 		    final long now = System.currentTimeMillis();
 
-		    for (Iterator i=set.iterator(); i.hasNext() && !dead; ) {
+		    for (Iterator<TransitionWatcher> i=set.iterator(); i.hasNext() && !dead; ) {
 			final TransitionWatcher watcher = 
-			    (TransitionWatcher)i.next();
+			    i.next();
 			watcher.process(t, now);
 		    }
 		} else if (payload instanceof CaughtUpMarker) {

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/OutriggerServerImpl.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/OutriggerServerImpl.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/OutriggerServerImpl.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/OutriggerServerImpl.java Mon Jun  3 22:01:38 2013
@@ -19,6 +19,7 @@ package com.sun.jini.outrigger;
 
 import au.net.zeus.collection.RC;
 import au.net.zeus.collection.Ref;
+import au.net.zeus.collection.Referrer;
 import com.sun.jini.config.Config;
 import com.sun.jini.constants.TimeConstants;
 import com.sun.jini.landlord.Landlord;
@@ -1241,7 +1242,7 @@ public class OutriggerServerImpl 
 	    grant(rep, lease, entryLeasePolicy, "entryLeasePolicy");
 
 	final EntryHolder holder = contents.holderFor(rep);
-	final EntryHandle handle = new EntryHandle(rep, txn, holder);
+	final EntryHandle handle = holder.newEntryHandle(rep, txn);
 
 	// Verify that the transaction is still active. Lock it so
 	// nobody can change it behind our backs while where making
@@ -1320,7 +1321,7 @@ public class OutriggerServerImpl 
 	    leaseData[i] = 
 		grant(entry, leaseTimes[i], entryLeasePolicy, "entryLeasePolicy");
 	    holders[i] = contents.holderFor(entry);
-	    handles[i] = new EntryHandle(entry, txn, holders[i]);
+	    handles[i] = holders[i].newEntryHandle(entry, txn);
 	}
 
 	// Verify that the transaction is still active. Lock it so
@@ -1469,7 +1470,7 @@ public class OutriggerServerImpl 
      */
      boolean attemptCapture(EntryHandle handle, TransactableMgr txn,
           boolean takeIt, Set lockedEntrySet, 
-	  Map provisionallyRemovedEntrySet, long now,
+	  Set<EntryHandle> provisionallyRemovedEntrySet, long now,
 	  QueryWatcher watcher) 
      {
 	 final EntryHolder holder = contents.holderFor(handle.rep());
@@ -1894,10 +1895,10 @@ public class OutriggerServerImpl 
 	/* First we do a straight search of the entries currently in the space */
 	   
 	// Set of classes we need to search
-	final Set classes = new java.util.HashSet();
+	final Set<String> classes = new java.util.HashSet<String>();
 	for (int i=0; i<tmpls.length; i++) {
 	    final String whichClass = tmpls[i].classFor();
-	    final Iterator subtypes = types.subTypes(whichClass);
+	    final Iterator<String> subtypes = types.subTypes(whichClass);
 	    while (subtypes.hasNext()) {
 		classes.add(subtypes.next());
 	    }		
@@ -1907,13 +1908,20 @@ public class OutriggerServerImpl 
 	EntryHandle[] handles = new EntryHandle[limit];
 	int found = 0;
 	final Set conflictSet = new java.util.HashSet();
-	final Map provisionallyRemovedEntrySet = 
-                RC.map(new ConcurrentHashMap(), Ref.WEAK_IDENTITY, Ref.STRONG, 10000L, 10000L);
+	final Set<EntryHandle> provisionallyRemovedEntrySet = 
+               Collections.newSetFromMap(
+                RC.map(
+                    new ConcurrentHashMap<Referrer<EntryHandle>,Referrer<Boolean>>(),
+                    Ref.WEAK_IDENTITY,
+                    Ref.STRONG,
+                    10000L,
+                    10000L
+                ));
 
-	for (Iterator i=classes.iterator(); 
+	for (Iterator<String> i=classes.iterator(); 
 	     i.hasNext() && found < handles.length;) 
         {
-	    final String clazz = (String)i.next();
+	    final String clazz = i.next();
 	    final EntryHolder.ContinuingQuery query = 
 		createQuery(tmpls, clazz, txn, true, start);
 
@@ -2144,7 +2152,7 @@ public class OutriggerServerImpl 
 	if (supertypes == null)
 	    return null;
 
-	final List tmplsToCheck = new java.util.LinkedList();	
+	final List<EntryRep> tmplsToCheck = new java.util.LinkedList<EntryRep>();	
 	for (int i=0; i<tmpls.length; i++) {
 	    final EntryRep tmpl = tmpls[i];
 	    final String tmplClass = tmpl.classFor();
@@ -2161,7 +2169,7 @@ public class OutriggerServerImpl 
 	}
 
 	return holder.continuingQuery(
-            (EntryRep[])tmplsToCheck.toArray(new EntryRep[tmplsToCheck.size()]),
+            tmplsToCheck.toArray(new EntryRep[tmplsToCheck.size()]),
 	    txn, takeIt, now);				       
     }
 
@@ -2171,13 +2179,13 @@ public class OutriggerServerImpl 
      * in the passed set.
      */
     private static void waitOnProvisionallyRemovedEntries(
-	    Map provisionallyRemovedEntrySet) 
+	    Set<EntryHandle> provisionallyRemovedEntrySet) 
 	throws InterruptedException
     {
 	if (provisionallyRemovedEntrySet.isEmpty())
 	    return;
 	
-	final Set keys = provisionallyRemovedEntrySet.keySet();
+	final Set<EntryHandle> keys = provisionallyRemovedEntrySet;
 
 	for (Iterator i=keys.iterator(); i.hasNext();) {
 	    final EntryHandle handle = (EntryHandle)i.next();
@@ -2269,12 +2277,19 @@ public class OutriggerServerImpl 
 	EntryHandle handle = null;
 	final Set conflictSet = new java.util.HashSet();
         // Shared by multiple new objects.
-	final Set lockedEntrySet = 
-	    (ifExists? Collections.newSetFromMap( new ConcurrentHashMap()):null);
+	final Set<Uuid> lockedEntrySet = 
+	    (ifExists? Collections.newSetFromMap( new ConcurrentHashMap<Uuid,Boolean>()):null);
         
         // Changed to concurrent map, because unsynchronized iteration occurs.
-	final Map provisionallyRemovedEntrySet = 
-	    RC.map(new ConcurrentHashMap(), Ref.WEAK_IDENTITY, Ref.STRONG, 10000L, 10000L);
+	final Set<EntryHandle> provisionallyRemovedEntrySet = 
+	    Collections.newSetFromMap(
+                RC.map(
+                    new ConcurrentHashMap<Referrer<EntryHandle>,Referrer<Boolean>>(),
+                    Ref.WEAK_IDENTITY,
+                    Ref.STRONG,
+                    10000L,
+                    10000L
+                ));
 
 	/*
 	 * First we do the straight search
@@ -2461,7 +2476,7 @@ public class OutriggerServerImpl 
      * Make sure the transactions listed here are monitored for as
      * long as the given query exists.
      */
-    private void monitor(QueryWatcher watcher, Collection toMonitor) {
+    private void monitor(QueryWatcher watcher, Collection<Txn> toMonitor) {
 	if (!toMonitor.isEmpty())	    
 	    txnMonitor.add(watcher, toMonitor);
     }
@@ -2471,7 +2486,7 @@ public class OutriggerServerImpl 
      * a reasonable amount of time since they recently caused a conflict,
      * although for a non-leased event.
      */
-    void monitor(Collection toMonitor) {
+    void monitor(Collection<Txn> toMonitor) {
 	if (!toMonitor.isEmpty())	    
 	    txnMonitor.add(toMonitor);
     }
@@ -2506,7 +2521,7 @@ public class OutriggerServerImpl 
      */
     private EntryHandle
 	find(EntryRep tmplRep, Txn txn, boolean takeIt, Set conflictSet, 
-	     Set lockedEntrySet, Map provisionallyRemovedEntrySet)
+	     Set lockedEntrySet, Set<EntryHandle> provisionallyRemovedEntrySet)
 	throws TransactionException
     {
 	final String whichClass = tmplRep.classFor();
@@ -2687,8 +2702,15 @@ public class OutriggerServerImpl 
 	 * Set of entries that we have encountered that have been
 	 * provisionally removed
 	 */
-	final private Map provisionallyRemovedEntrySet
-	    = RC.map(new ConcurrentHashMap(), Ref.WEAK_IDENTITY, Ref.STRONG, 10000L, 10000L) ;
+	final private Set<EntryHandle> provisionallyRemovedEntrySet
+	    = Collections.newSetFromMap(
+                RC.map(
+                    new ConcurrentHashMap<Referrer<EntryHandle>,Referrer<Boolean>>(),
+                    Ref.WEAK_IDENTITY,
+                    Ref.STRONG, 
+                    10000L, 
+                    10000L
+                )) ;
 
 	private ContentsQuery(Uuid uuid, EntryRep[] tmpls, Txn txn, long limit) {
 	    this.uuid = uuid;
@@ -3502,7 +3524,7 @@ public class OutriggerServerImpl 
 	typeCheck(rep);
 
 	final EntryHolder holder = contents.holderFor(rep);
-	final EntryHandle handle = new EntryHandle(rep, txn, holder);
+	final EntryHandle handle = holder.newEntryHandle(rep, txn);
 	addWrittenRep(handle, holder, txn);
     }
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ReadIfExistsWatcher.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ReadIfExistsWatcher.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ReadIfExistsWatcher.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ReadIfExistsWatcher.java Mon Jun  3 22:01:38 2013
@@ -18,6 +18,7 @@
 package com.sun.jini.outrigger;
 
 import java.util.Set;
+import net.jini.id.Uuid;
 
 /**
  * Subclass of <code>QueryWatcher</code> for non-transactional if
@@ -34,7 +35,7 @@ class ReadIfExistsWatcher extends Single
      * unavailable (e.g. they are locked). We only keep
      * the ids, not the entries themselves.
      */
-    private final Set lockedEntries;
+    private final Set<Uuid> lockedEntries;
 
     /**
      * Set <code>true</code> once the query thread is 
@@ -61,7 +62,7 @@ class ReadIfExistsWatcher extends Single
      *         <code>null</code>.
      */
     ReadIfExistsWatcher(long expiration, long timestamp, long startOrdinal, 
-			Set lockedEntries)
+			Set<Uuid> lockedEntries)
     {
 	super(expiration, timestamp, startOrdinal);
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TakeIfExistsWatcher.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TakeIfExistsWatcher.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TakeIfExistsWatcher.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TakeIfExistsWatcher.java Mon Jun  3 22:01:38 2013
@@ -21,6 +21,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.WeakHashMap;
 import net.jini.core.transaction.TransactionException;
+import net.jini.id.Uuid;
 import net.jini.space.InternalSpaceException;
 
 /**
@@ -37,7 +38,7 @@ class TakeIfExistsWatcher extends Single
      * unavailable (e.g. they are locked). We only keep
      * the ids, not the entries themselves.
      */
-    private final Set lockedEntries;
+    private final Set<Uuid> lockedEntries;
 
     /**
      * Set <code>true</code> once the query thread is 
@@ -59,7 +60,7 @@ class TakeIfExistsWatcher extends Single
      * we would have liked to return, but have been provisionally
      * removed.
      */
-    private final Map provisionallyRemovedEntrySet;
+    private final Set<EntryHandle> provisionallyRemovedEntrySet;
 
     /**
      * Create a new <code>TakeIfExistsWatcher</code>.
@@ -89,8 +90,8 @@ class TakeIfExistsWatcher extends Single
      *         <code>null</code>.
      */
     TakeIfExistsWatcher(long expiration, long timestamp, 
-	 long startOrdinal, Set lockedEntries, 
-         Map provisionallyRemovedEntrySet, Txn txn)
+	 long startOrdinal, Set<Uuid> lockedEntries, 
+         Set<EntryHandle> provisionallyRemovedEntrySet, Txn txn)
     {
 	super(expiration, timestamp, startOrdinal);
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TakeMultipleWatcher.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TakeMultipleWatcher.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TakeMultipleWatcher.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TakeMultipleWatcher.java Mon Jun  3 22:01:38 2013
@@ -46,7 +46,7 @@ class TakeMultipleWatcher extends QueryW
      * we would have liked to return, but have been provisionally
      * removed.
      */
-    private final Map provisionallyRemovedEntrySet;
+    private final Set<EntryHandle> provisionallyRemovedEntrySet;
 
     /**
      * If non-null the transaction this query is
@@ -107,7 +107,7 @@ class TakeMultipleWatcher extends QueryW
      *        a transaction the <code>Txn</code> object
      *        associated with that transaction.  */
     TakeMultipleWatcher(int limit, long expiration, long timestamp, 
-        long startOrdinal, Map provisionallyRemovedEntrySet, Txn txn)
+        long startOrdinal, Set<EntryHandle> provisionallyRemovedEntrySet, Txn txn)
     {
 	super(expiration, timestamp, startOrdinal);
 	this.limit = limit;

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TemplateHandle.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TemplateHandle.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TemplateHandle.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TemplateHandle.java Mon Jun  3 22:01:38 2013
@@ -18,10 +18,16 @@
 package com.sun.jini.outrigger;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Set;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Queue;
 import java.util.Vector;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
 
 /**
  * <code>TemplateHandle</code> associates one or more
@@ -29,14 +35,8 @@ import java.util.Vector;
  * Unless otherwise noted all methods are thread safe.
  */
 class TemplateHandle extends BaseHandle {
+    
     /**
-     * A cache of the <code>EntryHandleTmplDesc</code> indexed
-     * by the number of fields.
-     */
-    final private ArrayList<EntryHandleTmplDesc> descs = new ArrayList<EntryHandleTmplDesc>();
-
-    /**
-
      * The watchers. We use a <code>HashSet</code> because we will
      * probably do a fair number of removals for each traversal and
      * the number of watchers managed by one <code>TemplateHandle</code>
@@ -47,63 +47,39 @@ class TemplateHandle extends BaseHandle 
      * changing <code>FastList</code> to support overlapping traversals
      * of different lists from the same thread.)
      */
-    final private Set<TransitionWatcher> watchers = new java.util.HashSet<TransitionWatcher>();
+    final private Set<TransitionWatcher> watchers 
+            = Collections.newSetFromMap(
+                        new ConcurrentHashMap<TransitionWatcher,Boolean>());
+    /**
+     * WriteLock guarantees that no updates can be performed during a 
+     * removal operation.
+     */
+    final private WriteLock wl;
+    final private ReadLock rl;
+    private boolean removed; // mutate with wl, read with rl
 
     /**
      * The <code>WatchersForTemplateClass</code> this object
      * belongs to.
      */
-    final private WatchersForTemplateClass owner;
+    final private OutriggerServerImpl owner;
 
     /**
      * Create a handle for the template <code>tmpl</code>.
      */
-    TemplateHandle(EntryRep tmpl, WatchersForTemplateClass owner) {
-	super(tmpl);
+    TemplateHandle(EntryRep tmpl, OutriggerServerImpl owner, Queue<TemplateHandle> content) {
+	super(tmpl, content);
 	this.owner = owner;
+        ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();
+        wl = rwl.writeLock();
+        rl = rwl.readLock();
     }
 
     /**
      * Return the description for the given field count.
      */
-    //!! Since the mask/hash algorithm tops out at a certain number of fields,
-    //!! we could avoid some overhead by topping out at the same count.
     EntryHandleTmplDesc descFor(int index) {
-	/* Since setSize can truncate, test and set need to be atomic.
-	 * Hold the lock after setting the size so don't calculate
-	 * a given desc more than once (though that is only an optimization)
-	 */
-	synchronized (descs) {
-	    
-	    int size = descs.size();
-
-	    // Do we have a cached value?
-	    EntryHandleTmplDesc desc = null;
-            if (index < size) desc = descs.get(index);
-
-	    if (desc == null) {
-		// None in cache, calculate one
-		desc = EntryHandle.descFor(rep(), index);
-                if (index == size){
-                    descs.add(desc);
-                }
-                else if (index < size){
-                    descs.set(index, desc);
-                }
-                // Make sure descs is big enough and pad with null if
-                // fields are added out of order.
-                else if (index > size) {
-                    descs.ensureCapacity(index + 1);
-                    int difference = index - size;
-                    for (int i = 0; i < difference; i++){
-                        descs.add(null);
-                    }
-                    descs.add(desc);
-                }
-		assert descs.indexOf(desc) == index;
-	    }
-	    return desc;
-	}
+        return EntryHandle.descFor(rep(), index);
     }
 
     /**
@@ -115,22 +91,24 @@ class TemplateHandle extends BaseHandle 
 
     /** 
      * Add a watcher to this handle. Assumes that the handle has not
-     * been removed from its <code>FastList</code> and that
-     * the caller owns the lock on <code>this</code>.
+     * been removed.
      * @param watcher the watcher to be added.
+     * @return true if watcher is added, false otherwise.
      * @throws NullPointerException if watcher is <code>null</code>.
      */
-    void addTransitionWatcher(TransitionWatcher watcher) {
-	assert Thread.holdsLock(this) : 
-	    "addTransitionWatcher() called without lock";
-
-	if (watcher == null)
+    boolean addTransitionWatcher(TransitionWatcher watcher) {
+        if (watcher == null)
 	    throw new NullPointerException("Watcher can not be null");
-
-        // If thread holds lock during removal TransitionWatcher cannot be added
-        // so this assertion is unnecessary.
-	assert !removed() : "Added watcher to a removed TemplateHandle";
-	watchers.add(watcher);
+        rl.lock();
+        try {
+            if (removed) return false;
+            if (watcher.addTemplateHandle(this)) {
+                return watchers.add(watcher);
+            }
+            return false;
+        } finally {
+            rl.unlock();
+        }
     }
 
     /**
@@ -140,10 +118,15 @@ class TemplateHandle extends BaseHandle 
      * @param watcher the watcher to be removed.
      * @throws NullPointerException if watcher is <code>null</code>.
      */
-    synchronized void removeTransitionWatcher(TransitionWatcher watcher) {
+    void removeTransitionWatcher(TransitionWatcher watcher) {
 	if (watcher == null)
 	    throw new NullPointerException("Watcher can not be null");
-	watchers.remove(watcher);
+        rl.lock();
+        try {
+            watchers.remove(watcher);
+        } finally {
+            rl.unlock();
+        }
     }
 
     /**
@@ -158,16 +141,22 @@ class TemplateHandle extends BaseHandle 
      * @param ordinal The ordinal associated with <code>transition</code>.
      * @throws NullPointerException if either argument is <code>null</code>.
      */
-    synchronized void collectInterested(Set set, EntryTransition transition,
+    void collectInterested(Set<TransitionWatcher> set, EntryTransition transition,
 					long ordinal) 
     {
-	final Iterator i = watchers.iterator();
-	while (i.hasNext()) {
-	    final TransitionWatcher w = (TransitionWatcher)i.next();
-	    if (w.isInterested(transition, ordinal)) {
-		set.add(w);
-	    }
-	}
+        rl.lock();
+        try {
+            if (removed) return;
+            final Iterator i = watchers.iterator();
+            while (i.hasNext()) {
+                final TransitionWatcher w = (TransitionWatcher)i.next();
+                if (w.isInterested(transition, ordinal)) {
+                    set.add(w);
+                }
+            }
+        } finally {
+            rl.unlock();
+        }
     }
 
     /**
@@ -177,7 +166,7 @@ class TemplateHandle extends BaseHandle 
      * handle is part of.
      */
     OutriggerServerImpl getServer() {
-	return owner.getServer();
+	return owner;
     }
 
     /**
@@ -187,36 +176,57 @@ class TemplateHandle extends BaseHandle 
      *            milliseconds since the beginning of the epoch.
      */
     void reap(long now) {
-	/* This could take a while, instead of blocking all other
-	 * access, clone the contents of watchers and
-	 * iterate down the clone (we don't do this too often and
-	 * watchers should never be that big so a shallow copy
-	 * should not be that bad. If it does get bad may
-	 * need to switch to a FastList for watchers.
-	 * (we don't do this for collection of interested watchers
-	 * because calls to isInterested() are designed to be cheap,
-	 * calls to removeIfExpired() will grab locks and could
-	 * write to disk))
-	 */
-	final TransitionWatcher content[];
-	synchronized (this) {
-	    content = new TransitionWatcher[watchers.size()];
-	    watchers.toArray(content);
-	}
-
-	for (int i=0; i<content.length; i++) {
-	    content[i].removeIfExpired(now);
-	}
+        rl.lock();
+        try{
+            Iterator<TransitionWatcher> it = watchers.iterator();
+            while (it.hasNext()){
+                it.next().removeIfExpired(now);
+            }
+        } finally {
+            rl.unlock();
+        }
+    }
+
+    
+    /**
+     * Need to lock on the wl so no one will
+     * add a watcher between the check for empty and
+     * when it gets removed.
+     */
+    boolean removeIfEmpty(){
+        wl.lock();
+        try {
+            if (watchers.isEmpty()) {
+                return remove();
+            }
+            return false;
+        } finally {
+            wl.unlock();
+        }
+    }
+
+    @Override
+    public boolean removed() {
+        rl.lock();
+        try {
+            return removed;
+        } finally {
+            rl.unlock();
+        }
+    }
+
+    @Override
+    public boolean remove() {
+        wl.lock();
+        try {
+            if (removed){
+                return false; // already removed.
+            } else {
+                removed = super.remove();
+                return removed;
+            }
+        } finally {
+            wl.unlock();
+        }
     }
-
-    /**
-     * Return <code>true</code> if there are no watchers associated
-     * with this object and <code>false</code> otherwise. Assumes
-     * the call holds the lock on <code>this</code>.
-     * @return <code>true</code> if there are no watchers in this handle.
-     */
-    boolean isEmpty() {
-	assert Thread.holdsLock(this) : "isEmpty() called without lock";
-	return watchers.isEmpty();
-    } 
 }

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransactableReadIfExistsWatcher.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransactableReadIfExistsWatcher.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransactableReadIfExistsWatcher.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransactableReadIfExistsWatcher.java Mon Jun  3 22:01:38 2013
@@ -21,6 +21,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.WeakHashMap;
 import net.jini.core.transaction.TransactionException;
+import net.jini.id.Uuid;
 import net.jini.space.InternalSpaceException;
 
 /**
@@ -38,7 +39,7 @@ class TransactableReadIfExistsWatcher ex
      * unavailable (e.g. they are locked). We only keep
      * the ids, not the entries themselves.
      */
-    private final Set lockedEntries;
+    private final Set<Uuid> lockedEntries;
 
     /**
      * Set <code>true</code> once the query thread is 
@@ -59,7 +60,7 @@ class TransactableReadIfExistsWatcher ex
      * we would have liked to return, but have been provisionally
      * removed.
      */
-    private final Map provisionallyRemovedEntrySet;
+    private final Set<EntryHandle> provisionallyRemovedEntrySet;
 
     /**
      * Create a new <code>TransactableReadIfExistsWatcher</code>.
@@ -89,8 +90,8 @@ class TransactableReadIfExistsWatcher ex
      *         <code>txn</code> is <code>null</code>.
      */
     TransactableReadIfExistsWatcher(long expiration, long timestamp, 
-	 long startOrdinal, Set lockedEntries, 
-         Map provisionallyRemovedEntrySet, Txn txn)
+	 long startOrdinal, Set<Uuid> lockedEntries, 
+         Set<EntryHandle> provisionallyRemovedEntrySet, Txn txn)
     {
 	super(expiration, timestamp, startOrdinal);
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransitionWatcher.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransitionWatcher.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransitionWatcher.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransitionWatcher.java Mon Jun  3 22:01:38 2013
@@ -35,7 +35,7 @@ import java.util.concurrent.atomic.Atomi
  * Unless otherwise noted, all the package protected methods
  * are thread safe.  
  */
-abstract class TransitionWatcher implements Comparable {
+abstract class TransitionWatcher implements Comparable<TransitionWatcher> {
     /** The time stamp for this object */
     final private long timestamp;
 
@@ -102,8 +102,8 @@ abstract class TransitionWatcher impleme
      * @throws ClassCastException if <code>o</code> is not
      *         a <code>TransitionWatcher</code>.  
      */
-    public int compareTo(Object o) {
-	final TransitionWatcher other = (TransitionWatcher)o;
+    public int compareTo(TransitionWatcher o) {
+	final TransitionWatcher other = o;
 	if (timestamp < other.timestamp)
 	    return -1;
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransitionWatchers.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransitionWatchers.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransitionWatchers.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TransitionWatchers.java Mon Jun  3 22:01:38 2013
@@ -56,11 +56,18 @@ class TransitionWatchers {
      *        <code>null</code>
      */
     TransitionWatchers(OutriggerServerImpl server) {
-	if (server == null)
+	this(check(server), server);
+    }
+    
+    private static boolean check(OutriggerServerImpl server) throws NullPointerException {
+        if (server == null)
 	    throw new NullPointerException("server must be non-null");
-	this.server = server;
+        return true;
     }
 
+    private TransitionWatchers(boolean checked, OutriggerServerImpl server){
+        this.server = server;
+    }
     /**
      * Add a <code>TransitionWatcher</code> to the list
      * of watchers looking for visibility transitions in
@@ -88,7 +95,7 @@ class TransitionWatchers {
 	final String className = template.classFor();
 	WatchersForTemplateClass holder = holders.get(className);	    
         if (holder == null) {
-            holder = new WatchersForTemplateClass(this);
+            holder = new WatchersForTemplateClass(server);
             WatchersForTemplateClass existed = holders.putIfAbsent(className, holder);
             if (existed != null) holder = existed;
         }
@@ -123,9 +130,9 @@ class TransitionWatchers {
      * @throws NullPointerException if <code>transition</code> is 
      *         <code>null</code>.
      */
-    SortedSet allMatches(EntryTransition transition, long ordinal) {
+    SortedSet<TransitionWatcher> allMatches(EntryTransition transition, long ordinal) {
 	final EntryRep rep = transition.getHandle().rep();
-	final SortedSet rslt = new java.util.TreeSet();
+	final SortedSet<TransitionWatcher> rslt = new java.util.TreeSet<TransitionWatcher>();
 	final String className = rep.classFor();
 	WatchersForTemplateClass holder;
 	
@@ -153,22 +160,10 @@ class TransitionWatchers {
 
     /**
      * Visit each <code>TransitionWatcher</code> and check to see if
-     * it has expired, removing it if it has.  Also reap the
-     * <code>FastList</code>s.
+     * it has expired, removing it if it has. 
      */
     void reap() {
-        //Old comment related to cloning holders.values().
- 	/* This could take a while, instead of blocking all other
-	 * access to handles, clone the contents of handles and
-	 * iterate down the clone (we don't do this too often and
-	 * handles should never be that big so a shallow copy should
-	 * not be that bad, if it did get to be bad caching the
-	 * clone would probably work well since we don't add (and
-	 * never remove) elements that often.)
-	 */
-	
 	final long now = System.currentTimeMillis();
-	
         Iterator<WatchersForTemplateClass> watchers = holders.values().iterator();
         while (watchers.hasNext()){
             watchers.next().reap(now);

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/Txn.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/Txn.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/Txn.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/Txn.java Mon Jun  3 22:01:38 2013
@@ -112,7 +112,7 @@ class Txn implements TransactableMgr, Tr
      * The list of <code>Transactable</code> participating in 
      * this transaction.
      */
-    final private List txnables = new java.util.LinkedList();
+    final private List<Transactable> txnables = new java.util.LinkedList<Transactable>();
 
     /**
      * The task responsible for monitoring to see if this
@@ -277,11 +277,11 @@ class Txn implements TransactableMgr, Tr
 	    }
 
 	    // loop through Transactable members of this Txn
-	    final Iterator i = txnables.iterator();
+	    final Iterator<Transactable> i = txnables.iterator();
 	    int c=0; // Counter for debugging message
 	    while (i.hasNext()) {
 		// get this member's vote
-		final Transactable transactable = (Transactable)i.next();
+		final Transactable transactable = i.next();
 		final int prepState =  transactable.prepare(this, space);
 		if (logger.isLoggable(Level.FINEST)) {
 		    logger.log(Level.FINEST, "prepare:prepared " +
@@ -361,9 +361,10 @@ class Txn implements TransactableMgr, Tr
 		new Object[]{tr, Long.valueOf(trId), TxnConstants.getName(state)});
 	    }
 
-	    final Iterator i = txnables.iterator();
-	    while (i.hasNext())
-		((Transactable) i.next()).abort(this, space);
+	    final Iterator<Transactable> i = txnables.iterator();
+	    while (i.hasNext()) {
+                i.next().abort(this, space);
+            }
 	    state = ABORTED;
 	    cleanup();
 	    break;
@@ -400,15 +401,16 @@ class Txn implements TransactableMgr, Tr
 	    return;
 
 	  case PREPARED:	// voted PREPARED, time to finish up
-	    final Iterator i = txnables.iterator();
+	    final Iterator<Transactable> i = txnables.iterator();
 	    if (logger.isLoggable(Level.FINEST)) {
 		logger.log(Level.FINEST, "commit:committing transaction mgr:" +
 		    "{0}, id:{1}, state:{2}", 
 		new Object[]{tr, Long.valueOf(trId), TxnConstants.getName(state)});
 	    }
 
-	    while (i.hasNext())
-		((Transactable) i.next()).commit(this, space);
+	    while (i.hasNext()) {
+                i.next().commit(this, space);
+            }
 	    state = COMMITTED;
 	    cleanup();
 	    return;

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnMonitor.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnMonitor.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnMonitor.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnMonitor.java Mon Jun  3 22:01:38 2013
@@ -52,9 +52,9 @@ class TxnMonitor implements Runnable {
      */
     private static class ToMonitor {
 	final QueryWatcher	query;         // query governing interest in txns
-	final Collection	txns;	       // the transactions to monitor
+	final Collection<Txn>	txns;	       // the transactions to monitor
 
-	ToMonitor(QueryWatcher query, Collection txns) {
+	ToMonitor(QueryWatcher query, Collection<Txn> txns) {
 	    this.query = query;
 	    this.txns = txns;
 	}
@@ -71,7 +71,7 @@ class TxnMonitor implements Runnable {
      * @see OutriggerServerImpl#getMatch 
      */
     // @see #ToMonitor
-    private final LinkedList pending = new LinkedList();
+    private final LinkedList<ToMonitor> pending = new LinkedList<ToMonitor>();
 
     /** wakeup manager for <code>TxnMonitorTask</code>s */
     private final WakeupManager wakeupMgr = 
@@ -154,14 +154,14 @@ class TxnMonitor implements Runnable {
      * Add a set of <code>transactions</code> to be monitored under the
      * given query.
      */
-    synchronized void add(QueryWatcher query, Collection transactions) {
+    synchronized void add(QueryWatcher query, Collection<Txn> transactions) {
 	if (logger.isLoggable(Level.FINEST)) {
 	    final StringBuffer buf = new StringBuffer();
 	    buf.append("Setting up monitor for ");
 	    buf.append(query);
 	    buf.append(" toMonitor:");
 	    boolean notFirst = false;
-	    for (Iterator i=transactions.iterator(); i.hasNext();) {
+	    for (Iterator<Txn> i=transactions.iterator(); i.hasNext();) {
 		if (notFirst) {
 		    buf.append(",");
 		    notFirst = true;
@@ -179,7 +179,7 @@ class TxnMonitor implements Runnable {
      * Add a set of <code>transactions</code> to be monitored under no
      * lease.
      */
-    void add(Collection transactions) {
+    void add(Collection<Txn> transactions) {
 	add(null, transactions);
     }
 
@@ -194,21 +194,21 @@ class TxnMonitor implements Runnable {
 		synchronized (this) {
 		
 		    // Sleep if nothing is pending.
-		    while (pending.isEmpty() && !die)
-			wait();
+		    while (pending.isEmpty() && !die) {
+                        wait();
+                    }
 
-		    if (die)
-			return;
+		    if (die) return;
 
-		    tm = (ToMonitor)pending.removeFirst();
+		    tm = pending.removeFirst();
 		}
 
 		logger.log(Level.FINER, "creating monitor tasks for {0}",
 			   tm.query);
 
-		Iterator it = tm.txns.iterator();
+		Iterator<Txn> it = tm.txns.iterator();
 		while (it.hasNext()) {
-		    Txn txn = (Txn) it.next();
+		    Txn txn = it.next();
 		    TxnMonitorTask task = taskFor(txn);
 		    task.add(tm.query);
 		}

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnMonitorTask.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnMonitorTask.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnMonitorTask.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnMonitorTask.java Mon Jun  3 22:01:38 2013
@@ -303,7 +303,7 @@ class TxnMonitorTask extends RetryTask
 		if (queries == null)	// no resources, so nobody wants it
 		    return false;	// try again next time
 
-		Iterator it = queries.keySet().iterator();
+		Iterator<QueryWatcher> it = queries.keySet().iterator();
 		boolean foundNeed = false;
 
 		if (logger.isLoggable(Level.FINEST)) {
@@ -312,7 +312,7 @@ class TxnMonitorTask extends RetryTask
 		}
 
 		while (it.hasNext()) {
-		    QueryWatcher query = (QueryWatcher)it.next();
+		    QueryWatcher query = it.next();
 		    if (query == null)     // gone -- the map will reap it
 			continue;
 		    if (logger.isLoggable(Level.FINEST)) {
@@ -322,9 +322,9 @@ class TxnMonitorTask extends RetryTask
 				       Long.valueOf(query.getExpiration())});
 		    }
 
-		    if (query.getExpiration() < nextQuery.get() || 
-			query.isResolved())
-			it.remove();	// expired, so we don't care about it
+		    if (query.getExpiration() < nextQuery.get() || query.isResolved()) {
+                        it.remove();
+                    }	// expired, so we don't care about it
 		    else {
 			foundNeed = true;
 			break;

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnState.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnState.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnState.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnState.java Mon Jun  3 22:01:38 2013
@@ -43,18 +43,18 @@ class TxnState {
      * direct reference to the only manager for this handle, or a reference
      * to an <code>HashSet</code> with entries for each associated manager.
      */
-    private volatile Object		mgrs;
+    private Object mgrs;
 
     /**
      * The current state of the handle, such as <code>READ</code> or
      * <code>TAKE</code>.
      */
-    private volatile int			state;
+    private int state;
 
     /**
      * The holder the handle which owns this object is in
      */
-    final private EntryHolder	holder;
+    final private EntryHolder holder;
 
     /** Logger for logging information about entry matching */
     private static final Logger matchingLogger = 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TypeTree.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TypeTree.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TypeTree.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TypeTree.java Mon Jun  3 22:01:38 2013
@@ -21,9 +21,16 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Hashtable;
 import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Random;
+import java.util.Set;
 import java.util.Vector;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ConcurrentSkipListSet;
 
 /**
  * A type tree for entries.  It maintains, for each class, a list of
@@ -38,8 +45,17 @@ import java.util.Vector;
  * @see OutriggerServerImpl
  */
 class TypeTree {
-    /** For each type, a vector of known subtypes */
-    private final Hashtable<String,Vector> subclasses = new Hashtable<String,Vector>();
+    /** For each type, a set of known subtypes.
+     * 
+     * Note: This was originally Hashtable<String,Vector<String>>, although
+     * a synchronized Map<String<Set<String>> could have been used in place
+     * of the concurrent collections, on this occasion, the concurrent
+     * implementations were easier to implement and understand atomically.
+     * 
+     * The decision to use concurrent collections was not made for performance
+     * reasons, but rather simplicity of implementation, see addKnown method.
+     */
+    private final ConcurrentMap<String,Set<String>> subclasses = new ConcurrentHashMap<String,Set<String>>();
 
     /**
      * A generator used to randomize the order of iterator returns
@@ -54,38 +70,34 @@ class TypeTree {
 	net.jini.core.entry.Entry.class.getName();
 
     /**
-     * Return the vector of subclasses for the given class.
+     * Return the set of subclasses for the given class.
      */
-    private Vector classVector(String whichClass) {
-        synchronized (subclasses){
-	return (Vector) subclasses.get(whichClass);
-    }
+    private Set<String> classSet(String whichClass) {
+        return subclasses.get(whichClass);
     }
 
     /**
      * An iterator that will walk through a list of known types.
      */
     // @see #RandomizedIterator
-    private static abstract class TypeTreeIterator implements Iterator {
+    private static abstract class TypeTreeIterator<T> implements Iterator<T> {
 	protected int cursor;		// the current position in the list
-	protected final Object[] typearray;	// the list of types as an array
+	protected final T [] typearray;	// the list of types as an array
         
-        protected TypeTreeIterator (Object [] types){
+        protected TypeTreeIterator (T [] types){
             cursor = 0;
             typearray = types;
         }
 
 	// inherit doc comment
         public boolean hasNext() {
-            if (cursor < typearray.length)
-                return true;
- 
+            if (cursor < typearray.length) return true;
             return false;
         }
  
 	// inherit doc comment
-        public Object next() throws NoSuchElementException {
-            Object val = null;
+        public T next() throws NoSuchElementException {
+            T val = null;
  
             if (cursor >= typearray.length)
                 throw new NoSuchElementException("TypeTreeIterator: next");
@@ -117,7 +129,7 @@ class TypeTree {
      * maintains a randomized list of subtypes for the given
      * <code>className</code>, including the class itself.
      */
-    static class RandomizedIterator extends TypeTreeIterator {
+    private static class RandomizedIterator extends TypeTreeIterator<String> {
 	/**
 	 * Create a new <code>RandomizedIterator</code> for the given
 	 * class.
@@ -131,12 +143,12 @@ class TypeTree {
 	 * Traverse the given type tree and add to the list all the
 	 * subtypes encountered within.
 	 */
-	private static void walkTree(Collection children, Collection list, TypeTree tree) {
+	private static void walkTree(Collection<String> children, Collection<String> list, TypeTree tree) {
 	    if (children != null) {
 		list.addAll(children);
-	        Object[] kids = children.toArray();
+	        String[] kids = children.toArray(new String[children.size()]);
 		for (int i = 0; i< kids.length; i++) {
-		    walkTree(tree.classVector((String)kids[i]), list, tree);
+		    walkTree(tree.classSet(kids[i]), list, tree);
 		}
 	    }
 	}
@@ -145,9 +157,10 @@ class TypeTree {
 	 * Set up this iterator to walk over the subtypes of this class,
 	 * including the class itself.  It then randomizes the list.
 	 */
-	private static Object [] init(String className, TypeTree tree) {
-            Collection<String> types = new ArrayList<String>();
-            Object [] typearray;
+	private static String [] init(String className, TypeTree tree) {
+            // Use a Linked to avoid resizing.
+            Collection<String> types = new LinkedList<String>();
+            String [] typearray;
 	    if (className.equals(EntryRep.matchAnyClassName())) {
 		// handle "match any" specially" -- search from ROOT
 		// Simplification suggested by 
@@ -159,15 +172,15 @@ class TypeTree {
 	    }
 
 	    // add all subclasses
-	    walkTree(tree.classVector(className), types, tree);
+	    walkTree(tree.classSet(className), types, tree);
 
 	    // Convert it to an array and then randomize
-	    typearray = types.toArray();
+	    typearray = types.toArray(new String[types.size()]);
 	    int randnum = 0;
-	    Object tmpobj = null;
-
-	    for (int i = 0; i < typearray.length; i++) {
-		randnum = numgen.nextInt(typearray.length - i);
+	    String tmpobj = null;
+            int length = typearray.length;
+	    for (int i = 0; i < length; i++) {
+		randnum = numgen.nextInt(length - i);
 		tmpobj = typearray[i];
 		typearray[i] = typearray[randnum];
 		typearray[randnum] = tmpobj;
@@ -183,7 +196,7 @@ class TypeTree {
      * In other words, it returns the names of all classes that are
      * instances of the class that named, in a random ordering.
      */
-    Iterator subTypes(String className) {
+    Iterator<String> subTypes(String className) {
 	return new RandomizedIterator(className, this);
     }
 
@@ -234,21 +247,13 @@ class TypeTree {
      * Add the subclass to the list of known subclasses of this superclass.  
      */
     private boolean addKnown(String superclass, String subclass) {
-	Vector v;
-
-	synchronized (subclasses) {
-	    v = classVector(superclass);
-	    if (v == null) {
-		v = new Vector();
-		subclasses.put(superclass, v);
-	    }
-	}
-
-	synchronized (v) {
-	    if (v.contains(subclass))
-		return false;
-	    v.addElement(subclass);
-	}
-	return true;
+	Set<String> v;
+        v = classSet(superclass);
+        if (v == null) {
+            v = new ConcurrentSkipListSet<String>();
+            Set<String> existed = subclasses.putIfAbsent(superclass, v);
+            if (existed != null) v = existed; // discard new set.
+        }
+	return v.add(subclass);
     }
 }

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/WatchersForTemplateClass.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/WatchersForTemplateClass.java?rev=1489197&r1=1489196&r2=1489197&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/WatchersForTemplateClass.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/WatchersForTemplateClass.java Mon Jun  3 22:01:38 2013
@@ -30,22 +30,20 @@ import java.util.concurrent.ConcurrentLi
  */
 class WatchersForTemplateClass {
     /** All the templates we know about */
-//    private final FastList<TemplateHandle> contents = new FastList<TemplateHandle>();	
-    
     private final Queue<TemplateHandle> content = new ConcurrentLinkedQueue<TemplateHandle>();
 
-    /** The object we are inside of */
-    private final TransitionWatchers owner;
+    /** The OutriggerServerImpl we belong to */
+    private final OutriggerServerImpl owner;
 
     /**
      * Create a new <code>WatchersForTemplateClass</code> object
      * associated with the specified <code>TransitionWatchers</code> object.
-     * @param owner The <code>TransitionWatchers</code> that
+     * @param owner The <code>OutriggerServerImpl</code> that
      *              this object will be a part of.
      * @throws NullPointerException if <code>owner</code> is
      *         <code>null</code>.
      */
-    WatchersForTemplateClass(TransitionWatchers owner) {
+    WatchersForTemplateClass(OutriggerServerImpl owner) {
 	if (owner == null)
 	    throw new NullPointerException("owner must be non-null");
 	this.owner = owner;
@@ -66,52 +64,18 @@ class WatchersForTemplateClass {
      */
     void add(TransitionWatcher watcher, EntryRep template) {
 	/* We try to find an existing handle, but it is ok
-	 * if we have more than one with the same template. It
-	 * is bad if we add the watcher to a removed handle.
+	 * if we have more than one with the same template.
+         * It isn't possible to add a watcher to a removed
+         * handle even if present during iteration.
 	 */	
         for(TemplateHandle handle : content) {
-	    if (template.equals(handle.rep())) {
-		synchronized (handle) {
-		    if (!handle.removed()) {
-			/* Found one, add and break. Call
-			 * addTemplateHandle() before adding to handle
-			 * so if the handle calls the watcher it will
-			 * be in a complete state. Add inside the
-			 * lock so handle can't be removed.
-			 */
-			if (watcher.addTemplateHandle(handle)) {
-			    handle.addTransitionWatcher(watcher);
-			} // else the watcher was removed, don't add to handle
-
-			return; // found a handle and added the watcher
-		    }
-		}
-	    }
-	}
-
-	/* If we are here we could not find a handle with the right
-	 * template, create one, add the watcher to it, and it
-	 * to contents.
-	 */
-	TemplateHandle handle = new TemplateHandle(template, this);
-
-	/* We need the sync both to prevent concurrent modification
-	 * of handle (since we add it to contents first), and to
-	 * make sure other threads see the changes we are about to make.
-	 */
-	synchronized (handle) {
-	    /* First add handle to contents so handle is fully initialized 
-	     * before we start to pass it around use
-	     */
-	    content.add(handle);
-	    if (watcher.addTemplateHandle(handle)) {
-		handle.addTransitionWatcher(watcher);
-	    } else {
-		// watcher is already dead, undo adding handle
-		content.remove(handle);	
-                handle.remove();
-	    }
+	    if (template.equals(handle.rep()) &&
+                handle.addTransitionWatcher(watcher)) return;
 	}
+        
+	TemplateHandle handle = new TemplateHandle(template, owner, content);
+        if (handle.addTransitionWatcher(watcher)) content.add(handle);
+        // else the new handle is discarded.
     }
     
     /**
@@ -126,7 +90,7 @@ class WatchersForTemplateClass {
      * @param ordinal The ordinal associated with <code>transition</code>.
      * @throws NullPointerException if either argument is <code>null</code>.
      */
-    void collectInterested(Set set, EntryTransition transition, 
+    void collectInterested(Set<TransitionWatcher> set, EntryTransition transition, 
 			   long ordinal) 
     {
 	final EntryHandle entryHandle = transition.getHandle();
@@ -145,26 +109,11 @@ class WatchersForTemplateClass {
 
 	    if ((entryHash & desc.mask) != desc.hash) continue;
 
-	    if (handle.matches(rep)) { // rep access is synchronized
-                synchronized (handle){
-                    if (handle.removed()) continue;
-                    handle.collectInterested(set, transition, ordinal);
-                }
-	    }
+	    if (handle.matches(rep)) handle.collectInterested(set, transition, ordinal);
 	}
     }
 
     /**
-     * Return the <code>OutriggerServerImpl</code> this 
-     * handle is part of.
-     * @return The <code>OutriggerServerImpl</code> this 
-     * handle is part of.
-     */
-    OutriggerServerImpl getServer() {
-	return owner.getServer();
-    }
-
-    /**
      * Visit each <code>TransitionWatcher</code> and check to see if
      * it has expired, removing it if it has. Also reaps the
      * <code>FastList</code> associated with this object.
@@ -177,24 +126,8 @@ class WatchersForTemplateClass {
 	{
 	    // Dump any expired watchers.
 	    handle.reap(now);
-
-	    /* Need to lock on the handle so no one will
-	     * add a watcher between the check for empty and
-	     * when it gets marked removed.
-	     */
-	    synchronized (handle) {
-		if (handle.isEmpty()) {
-		    content.remove(handle);
-                    handle.remove();
-		    continue;
-		}
-	    }
+            handle.removeIfEmpty();
 	}
-
-	/* This provides the FastList with an opportunity to actually
-	 * excise the items identified as "removed" from the list.
-	 */
-//	contents.reap();
     }
 }