river-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From peter_firmst...@apache.org
Subject svn commit: r1453746 - in /river/jtsk/skunk/qa_refactor/trunk/src: com/sun/jini/jeri/internal/runtime/ com/sun/jini/logging/ com/sun/jini/outrigger/ net/jini/activation/ net/jini/config/ net/jini/jeri/ net/jini/jeri/ssl/ net/jini/security/ org/apache/r...
Date Thu, 07 Mar 2013 09:38:10 GMT
Author: peter_firmstone
Date: Thu Mar  7 09:38:09 2013
New Revision: 1453746

URL: http://svn.apache.org/r1453746
Log:
Ran FindBugs, fixed numerous minor visibility race condition bugs, along with some other identified
bugs.

Modified:
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/jeri/internal/runtime/AbstractDgcClient.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/logging/Levels.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/AvailabilityRegistrationWatcher.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EventRegistrationWatcher.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ExpirationOpQueue.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/SingletonQueryWatcher.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/SpaceProxy2.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/StorableAvailabilityWatcher.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/StorableReference.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/Txn.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnTable.java
    river/jtsk/skunk/qa_refactor/trunk/src/net/jini/activation/ActivatableInvocationHandler.java
    river/jtsk/skunk/qa_refactor/trunk/src/net/jini/config/ConfigurationFile.java
    river/jtsk/skunk/qa_refactor/trunk/src/net/jini/jeri/BasicInvocationHandler.java
    river/jtsk/skunk/qa_refactor/trunk/src/net/jini/jeri/ssl/SslServerEndpointImpl.java
    river/jtsk/skunk/qa_refactor/trunk/src/net/jini/security/AuthenticationPermission.java
    river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/io/Distributed.java
    river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PolicyUtils.java

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/jeri/internal/runtime/AbstractDgcClient.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/jeri/internal/runtime/AbstractDgcClient.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/jeri/internal/runtime/AbstractDgcClient.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/jeri/internal/runtime/AbstractDgcClient.java
Thu Mar  7 09:38:09 2013
@@ -333,8 +333,7 @@ abstract class AbstractDgcClient {
 	 * This method must ONLY be invoked while synchronized on this
 	 * EndpointEntry.
 	 */
-	private void removeRefEntry(RefEntry refEntry) {
-	    assert Thread.holdsLock(this);
+	private synchronized void removeRefEntry(RefEntry refEntry) {
 	    assert !removed;
 	    assert refTable.containsKey(refEntry.getObjectID());
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/logging/Levels.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/logging/Levels.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/logging/Levels.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/logging/Levels.java Thu Mar  7 09:38:09
2013
@@ -20,6 +20,7 @@ package com.sun.jini.logging;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.EOFException;
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
@@ -28,6 +29,7 @@ import java.io.ObjectStreamException;
 import java.io.OutputStream;
 import java.io.Serializable;
 import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /**
 * Defines additional {@link Level} values. <p>
@@ -128,25 +130,31 @@ public class Levels {
     * See River-416 for details, the serial form of Levels was broken in Java 1.6.0_41.
     */
     private static Level createLevel(String name, int value, String resourceBundleName) {
+        Level result = null;
         try {
             ByteArrayOutputStream bytes = new ByteArrayOutputStream();
             ObjectOutputStream out = new ClassReplacingObjectOutputStream(bytes, LevelData.class,
Level.class);
             out.writeObject(new LevelData(name, value, resourceBundleName));
             out.close();
             ObjectInputStream in = new ObjectInputStream(new ByteArrayInputStream(bytes.toByteArray()));
-            Level result = (Level) in.readObject();
+            result = (Level) in.readObject();
             in.close();
             // If this suceeds, Level.readResolve has added the new Level to its internal
List.
+            
+        } catch (ClassNotFoundException ex) {
+            // Ignore :)
+        } catch (IOException e) {
+            // Ignore :)
+        } finally {
+            if (result == null){
+                final Level withoutName = Level.parse(Integer.valueOf(value).toString());
+                result =  new Level(name, value, resourceBundleName) {
+                    Object writeReplace() throws ObjectStreamException {
+                        return withoutName;
+                    }
+                };
+            }
             return result;
-        } catch (Exception e) {
-            final Level withoutName = Level.parse(Integer.valueOf(value).toString());
-            Level l =  new Level(name, value, resourceBundleName) {
-                Object writeReplace() throws ObjectStreamException {
-                    return withoutName;
-                }
-            };
-            return l;
         }
-        
     }
 }

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/AvailabilityRegistrationWatcher.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/AvailabilityRegistrationWatcher.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/AvailabilityRegistrationWatcher.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/AvailabilityRegistrationWatcher.java
Thu Mar  7 09:38:09 2013
@@ -41,21 +41,21 @@ abstract class AvailabilityRegistrationW
      * The current expiration time of the registration
      * Protected, but only for use by subclasses.
      */
-    long expiration;
+    volatile long expiration;
 
     /** 
      * The UUID that identifies this registration 
      * Only for use by subclasses.
      * Should not be changed.
      */
-    Uuid cookie;
+    volatile Uuid cookie;
 
     /**
      * The handback associated with this registration.
      * Only for use by subclasses.
      * Should not be changed.
      */
-    MarshalledObject handback;
+    volatile MarshalledObject handback;
 
     /**
      * <code>true</code> if client is interested
@@ -64,14 +64,14 @@ abstract class AvailabilityRegistrationW
      * Only for use by subclasses.
      * Should not be changed.
      */
-    boolean visibilityOnly;
+    volatile boolean visibilityOnly;
 
     /** 
      * The event ID associated with this registration
      * Protected, but only for use by subclasses.
      * Should not be changed.
      */
-    long eventID;
+    volatile long eventID;
 
     /** 
      * The current sequence number. 
@@ -82,7 +82,7 @@ abstract class AvailabilityRegistrationW
      * The <code>TemplateHandle</code>s associated with this
      * watcher.
      */
-    private Set owners = new java.util.HashSet();
+    private Set<TemplateHandle> owners = new java.util.HashSet<TemplateHandle>();
 
     /**
      * The OutriggerServerImpl we are part of.
@@ -282,7 +282,8 @@ abstract class AvailabilityRegistrationW
      *         <code>doIt</code> is <code>false</code>.
      */
     private boolean doRemove(long now, boolean doIt) {
-	final Set owners;
+	final Set<TemplateHandle> owners;
+        OutriggerServerImpl serv;
 	synchronized (this) {
 	    if (this.owners == null)
 		return false; // already removed
@@ -291,19 +292,19 @@ abstract class AvailabilityRegistrationW
 	    if (!doIt && (now < expiration))
 		return false; // don't remove, not our time
 
-	    owners = this.owners;
+	    owners = this.owners; // Don't need to clone
+            this.owners = null; // now it's null, it doesn't need sync anymore.
 	    expiration = Long.MIN_VALUE; //Make sure no one tries to renew us
-	    this.owners = null;
+            serv = server;
 	}
 
-	cleanup(server, !doIt);
-
-	for (Iterator i=owners.iterator(); i.hasNext(); ) {
-	    final TemplateHandle h = (TemplateHandle)i.next();
-	    h.removeTransitionWatcher(this);
-	}
-
-	server.removeEventRegistration(this);	    	
+	cleanup(serv, !doIt);
+        for (Iterator<TemplateHandle> i=owners.iterator(); i.hasNext(); ) {
+            final TemplateHandle h = i.next();
+            h.removeTransitionWatcher(this);
+        }
+        
+	serv.removeEventRegistration(this);	    	
 	return true; // we did the deed
     }
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EventRegistrationWatcher.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EventRegistrationWatcher.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EventRegistrationWatcher.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/EventRegistrationWatcher.java
Thu Mar  7 09:38:09 2013
@@ -39,7 +39,7 @@ abstract class EventRegistrationWatcher 
      * The current expiration time of the registration
      * Protected, but only for use by subclasses.
      */
-    long expiration;
+    volatile long expiration;
 
     /** 
      * The UUID that identifies this registration 
@@ -210,11 +210,12 @@ abstract class EventRegistrationWatcher 
 	if (h == null) 
 	    throw 
 		new NullPointerException("TemplateHandle must be non-null");
+        synchronized (this){
+            if (owner != null)
+                throw new AssertionError("Can only call addTemplateHandle once");
 
-	if (owner != null)
-	    throw new AssertionError("Can only call addTemplateHandle once");
-
-	owner = h;
+            owner = h;
+        }
 	return true;
     }
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ExpirationOpQueue.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ExpirationOpQueue.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ExpirationOpQueue.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/ExpirationOpQueue.java Thu
Mar  7 09:38:09 2013
@@ -28,7 +28,7 @@ import net.jini.id.Uuid;
  */
 class ExpirationOpQueue extends Thread {
     /** <code>true</code> if we should stop */
-    private boolean dead;
+    private volatile boolean dead;
 
     /** The queue of expirations to log */
     private final LinkedList queue = new LinkedList();

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=1453746&r1=1453745&r2=1453746&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 Thu
Mar  7 09:38:09 2013
@@ -61,7 +61,7 @@ class OperationJournal extends Thread {
     private JournalNode lastProcessed;
 
     /** If <code>true</code> stop thread */
-    private boolean dead = false;
+    private volatile boolean dead = false;
 
     /** The last ordinal value used */
     private long lastOrdinalUsed = 1;
@@ -314,7 +314,7 @@ class OperationJournal extends Thread {
      * @throws NullPointerException if <code>transition</code> is
      *         <code>null</code>.
      */
-    synchronized void recordTransition(EntryTransition transition) {
+    void recordTransition(EntryTransition transition) {
 	if (transition == null)
 	    throw new NullPointerException("transition must be non-null");
 
@@ -329,7 +329,7 @@ class OperationJournal extends Thread {
      * @throws NullPointerException if <code>watcher</code> is 
      *         <code>null</code>.     
      */
-    synchronized void markCaughtUp(IfExistsWatcher watcher) {
+    void markCaughtUp(IfExistsWatcher watcher) {
 	post(new JournalNode(new CaughtUpMarker(watcher)));
     }
 
@@ -347,10 +347,12 @@ class OperationJournal extends Thread {
      * Post a <code>JournalNode</code> 
      * @param node The node to post.
      */
-    private void post(JournalNode node) {
-	tail.next = node;
-	tail = node;
-	notifyAll();
+    private synchronized void post(JournalNode node) {
+        synchronized (tail){
+            tail.next = node;
+        }
+        tail = node;
+        notifyAll();
     }
 
     /**
@@ -414,6 +416,8 @@ class OperationJournal extends Thread {
 	while (!dead) {
 	    try {
 		// Wait until there is something to process
+                final Object payload;
+                long ordinal;
 		synchronized (this) {
 		    JournalNode n = lastProcessed.getNext();
 		    while (n == null && !dead) {
@@ -425,10 +429,12 @@ class OperationJournal extends Thread {
 			return;
 
 		    lastProcessed = n;
+                    // Process based on payload
+                    payload = lastProcessed.payload;
+                    ordinal = lastProcessed.ordinal;
 		}
 
-		// Process based on payload
-		final Object payload = lastProcessed.payload;
+		
 
 		if (payload == null) {
 		    throw new 
@@ -436,7 +442,7 @@ class OperationJournal extends Thread {
 		} else if (payload instanceof EntryTransition) {
 		    final EntryTransition t = (EntryTransition)payload;
 		    final SortedSet set = 
-			watchers.allMatches(t, lastProcessed.ordinal);
+			watchers.allMatches(t, ordinal);
 		    final long now = System.currentTimeMillis();
 
 		    for (Iterator i=set.iterator(); i.hasNext() && !dead; ) {

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/SingletonQueryWatcher.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/SingletonQueryWatcher.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/SingletonQueryWatcher.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/SingletonQueryWatcher.java
Thu Mar  7 09:38:09 2013
@@ -30,7 +30,7 @@ package com.sun.jini.outrigger;
  */
 abstract class SingletonQueryWatcher extends QueryWatcher {
     /** Set to true when this query is resolved */
-    private boolean resolved = false;
+    private volatile boolean resolved = false;
 
     /** If resolved and an entry was found the entry to return */
     private EntryHandle handle;

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/SpaceProxy2.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/SpaceProxy2.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/SpaceProxy2.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/SpaceProxy2.java Thu Mar
 7 09:38:09 2013
@@ -294,9 +294,15 @@ class SpaceProxy2 implements JavaSpace05
 	if (entry == null)
 	    throw new NullPointerException("Cannot write null Entry");
 	long[] leaseData = space.write(repFor(entry), txn, lease);
-	if (leaseData == null || leaseData.length != 3)
-	    throw new AssertionError("space.write returned malformed data" + 
-				     leaseData);
+	if (leaseData == null || leaseData.length != 3){
+            StringBuilder sb = new StringBuilder(180);
+            sb.append("space.write returned malformed data \n");
+            int l = leaseData.length;
+            for (int i =0; i < l; i++){
+                sb.append(leaseData[i]).append("\n");
+            }
+	    throw new AssertionError(sb);
+        }
 	return newLease(UuidFactory.create(leaseData[1], leaseData[2]),
 			leaseData[0]);
     }

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/StorableAvailabilityWatcher.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/StorableAvailabilityWatcher.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/StorableAvailabilityWatcher.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/StorableAvailabilityWatcher.java
Thu Mar  7 09:38:09 2013
@@ -120,7 +120,11 @@ class StorableAvailabilityWatcher extend
     RemoteEventListener getListener(ProxyPreparer preparer) 
 	throws ClassNotFoundException, IOException
     {
-	return (RemoteEventListener)listener.get(preparer);
+        StorableReference listen;
+        synchronized (this){
+            listen = listener;
+        }
+	return (RemoteEventListener)listen.get(preparer);
     }
 
     /**
@@ -133,22 +137,24 @@ class StorableAvailabilityWatcher extend
      *        <code>removeIfExpired</code> and false otherwise. 
      */
     void cleanup(OutriggerServerImpl server, boolean expired) {
-	if (expired)
-	    server.scheduleCancelOp(cookie);
-	else 
-	    server.cancelOp(cookie, false);
+        if (expired)
+            server.scheduleCancelOp(cookie);
+        else 
+            server.cancelOp(cookie, false);
     }
 
         /**  
      * Store the persistent fields 
      */
     public void store(ObjectOutputStream out) throws IOException {
-	cookie.write(out);
-	out.writeLong(expiration);
-	out.writeLong(eventID);
-	out.writeBoolean(visibilityOnly);
-	out.writeObject(handback);
-	out.writeObject(listener);
+        synchronized (this){
+            cookie.write(out);
+            out.writeLong(expiration);
+            out.writeLong(eventID);
+            out.writeBoolean(visibilityOnly);
+            out.writeObject(handback);
+            out.writeObject(listener);
+        }
     }
 
     /**
@@ -157,15 +163,17 @@ class StorableAvailabilityWatcher extend
     public void restore(ObjectInputStream in) 
 	throws IOException, ClassNotFoundException 
     {
-	cookie = UuidFactory.read(in);
-	expiration = in.readLong();
-	eventID = in.readLong();
-	visibilityOnly = in.readBoolean();
-	handback = (MarshalledObject)in.readObject();	
-	listener = (StorableReference)in.readObject();
-	if (listener == null)
-	    throw new StreamCorruptedException(
-		"Stream corrupted, should not be null");
+        synchronized (this){
+            cookie = UuidFactory.read(in);
+            expiration = in.readLong();
+            eventID = in.readLong();
+            visibilityOnly = in.readBoolean();
+            handback = (MarshalledObject)in.readObject();	
+            listener = (StorableReference)in.readObject();
+            if (listener == null)
+                throw new StreamCorruptedException(
+                    "Stream corrupted, should not be null");
+        }
     }
 }
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/StorableReference.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/StorableReference.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/StorableReference.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/StorableReference.java Thu
Mar  7 09:38:09 2013
@@ -118,15 +118,16 @@ class StorableReference implements Exter
 	synchronized (this) {
 	    if (bytes == null)
 		bytes = new MarshalledObject(obj);
+            out.writeObject(bytes);
 	}
-
-	out.writeObject(bytes);
     }
 
     // inherit doc comment
     public void readExternal(ObjectInput in)
 	throws IOException, ClassNotFoundException 
     {
-	bytes = (MarshalledObject)in.readObject();
+        synchronized (this){
+            bytes = (MarshalledObject)in.readObject();
+        }
     }
 }

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=1453746&r1=1453745&r2=1453746&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
Thu Mar  7 09:38:09 2013
@@ -55,7 +55,7 @@ class TakeMultipleWatcher extends QueryW
     private final Txn txn;
 
     /** Set to true when this query is resolved */
-    private boolean resolved = false;
+    private volatile boolean resolved = false;
 
     /** 
      * If resolved and an exception needs to be thrown the exception

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=1453746&r1=1453745&r2=1453746&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 Thu Mar  7 09:38:09
2013
@@ -91,7 +91,7 @@ class Txn implements TransactableMgr, Tr
     final private long id;
 
     /** What state we think the transaction is in */
-    private int	state;
+    private volatile int state;
 
     /** 
      * The transaction manager associated with the transaction 
@@ -106,7 +106,7 @@ class Txn implements TransactableMgr, Tr
     private ServerTransaction tr;
     
     /** The id the transaction manager assigned to this transaction */
-    private long  trId;
+    private volatile long  trId;
 
     /** 
      * The list of <code>Transactable</code> participating in 
@@ -430,12 +430,14 @@ class Txn implements TransactableMgr, Tr
     public ServerTransaction getTransaction(ProxyPreparer preparer)
 	throws IOException, ClassNotFoundException
     {
-	if (tr == null) {
-	    final TransactionManager mgr = 
-		(TransactionManager)trm.get(preparer);
-	    tr = new ServerTransaction(mgr, trId);
-	}
-	return tr;
+        synchronized (this){
+            if (tr == null) {
+                final TransactionManager mgr = 
+                    (TransactionManager)trm.get(preparer);
+                tr = new ServerTransaction(mgr, trId);
+            }
+            return tr;
+        }
     }
 
     /**
@@ -444,7 +446,7 @@ class Txn implements TransactableMgr, Tr
      * @throws IllegalStateException if this <code>Txn</code>
      *         is still broken.
      */
-    TransactionManager getManager() {
+    synchronized TransactionManager getManager() {
 	if (tr == null)
 	    throw new IllegalStateException("Txn is still broken");
 	return tr.mgr;
@@ -490,8 +492,10 @@ class Txn implements TransactableMgr, Tr
 	 * because it is always ACTIVE when we write, and always
 	 * needs to be PREPARED when we read it back.
 	 */
-	out.writeObject(trm);
-	out.writeLong(trId);
+        synchronized (this){
+            out.writeObject(trm);
+            out.writeLong(trId);
+        }
     }
 
     // inherit doc comment
@@ -501,8 +505,10 @@ class Txn implements TransactableMgr, Tr
 	/* Only transactions that got prepared and not committed or
 	 * aborted get recovered
 	 */
-	state    = PREPARED;
-	trm      = (StorableReference)in.readObject();
-	trId     = in.readLong();
+        synchronized (this){
+            state    = PREPARED;
+            trm      = (StorableReference)in.readObject();
+            trId     = in.readLong();
+        }
     }
 }

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnTable.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnTable.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnTable.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/outrigger/TxnTable.java Thu Mar  7
09:38:09 2013
@@ -28,6 +28,8 @@ import net.jini.core.transaction.server.
 import net.jini.security.ProxyPreparer;
 
 import com.sun.jini.logging.Levels;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 /**
  * Keeps a mapping from {@link TransactionManager}/id pairs, to {@link Txn}
@@ -115,14 +117,14 @@ class TxnTable {
 	 * <code>Txn</code>s. Only <code>Key</code>s that have their prepared
flag
 	 * set should go in this Map.
 	 */
-	final private Map txns = new java.util.HashMap();
+	final private ConcurrentMap<Key,Txn> txns = new ConcurrentHashMap<Key,Txn>();
 
 	/**
 	 * Map of transaction ids to the <code>List</code> of broken
 	 * <code>Txn</code> objects that have the id. <code>null</code>
if there are
 	 * no broken <code>Txn</code>s.
 	 */
-	private Map brokenTxns = null;
+	private final ConcurrentMap<Long,List<Txn>> brokenTxns = new ConcurrentHashMap<Long,List<Txn>>();
 
 	/**
 	 * <code>ProxyPreparer</code> to use when unpacking transactions, may be
@@ -180,34 +182,33 @@ class TxnTable {
 		final Long idAsLong;
 		final Txn brokenTxnsForId[];
 		// Try the table of non-broken txns first
-		synchronized (this) {
-			final Txn r = (Txn) txns.get(new Key(manager, id, false));
-			if (r != null)
-				return r;
-
-			// Check broken txns
-			if (brokenTxns == null)
-				// No broken Txns so txns is definitive
-				return null;
-
-			idAsLong = Long.valueOf(id);
-			final List txnsForId = (List) brokenTxns.get(idAsLong);
-			if (txnsForId == null)
-				/*
-				 * Broken Txns, but none with the right ID so txns is definitive
-				 * for this manager/id pair.
-				 */
-				return null;
-
-			/*
-			 * If we are here there are broken Txns with the specified id, we
-			 * need to try and fix each one and check to see if there is a
-			 * match. Make a copy of the list so we don't need to hold a lock
-			 * while making fix attempts (which in general involve remote
-			 * communications).
-			 */
-			brokenTxnsForId = (Txn[]) txnsForId.toArray(txnArray);
-		}
+                {
+                    final Txn r = (Txn) txns.get(new Key(manager, id, false));
+                    if (r != null) return r;
+
+                    // Check broken txns
+                    if (brokenTxns.isEmpty()) return null;// No broken Txns so txns is definitive
+                           
+                    idAsLong = Long.valueOf(id);
+                    final List txnsForId = (List) brokenTxns.get(idAsLong);
+                    if (txnsForId == null)
+                            /*
+                             * Broken Txns, but none with the right ID so txns is definitive
+                             * for this manager/id pair.
+                             */
+                            return null;
+
+                    /*
+                     * If we are here there are broken Txns with the specified id, we
+                     * need to try and fix each one and check to see if there is a
+                     * match. Make a copy of the list so we don't need to hold a lock
+                     * while making fix attempts (which in general involve remote
+                     * communications).
+                     */
+                    synchronized (txnsForId){
+                        brokenTxnsForId = (Txn[]) txnsForId.toArray(txnArray);
+                    }
+                }
 
 		/*
 		 * try and fix each element of brokenTxnsForId until we find the right
@@ -215,7 +216,7 @@ class TxnTable {
 		 */
 		Txn match = null;
 		Throwable t = null; // The first throwable we get, if any
-		final List fixed = new java.util.LinkedList(); // fixed Txns
+		final List<Txn> fixed = new java.util.LinkedList<Txn>(); // fixed Txns
 		for (int i = 0; i < brokenTxnsForId.length; i++) {
 			try {
 				final ServerTransaction st = brokenTxnsForId[i]
@@ -250,42 +251,40 @@ class TxnTable {
 		}
 
 		/*
-		 * Now that all the remote operations are done, re-acquire lock so we
+		 * Now that all the remote operations are done we
 		 * can move anything that was fixed.
 		 */
 		if (!fixed.isEmpty()) {
-			synchronized (this) {
-				if (brokenTxns != null) {
-					final List txnsForId = (List) brokenTxns.get(idAsLong);
-					if (txnsForId != null) {
-
-						// Remove from brokenTxns
-						txnsForId.removeAll(fixed);
-
-						// can we get rid of txnsForId and brokenTxns?
-						if (txnsForId.isEmpty()) {
-							brokenTxns.remove(idAsLong);
-							if (brokenTxns.isEmpty())
-								brokenTxns = null;
-						}
-
-						// Put in txns
-						for (Iterator i = fixed.iterator(); i.hasNext();) {
-							put((Txn) i.next());
-						}
-					} else {
-						/*
-						 * if the list for this id is gone the fixed Txns must
-						 * have already been moved by someone else
-						 */
-					}
-				} else {
-					/*
-					 * if brokenTxns is gone the fixed Txns must have already
-					 * been moved by someone else
-					 */
-				}
-			}
+			
+                    if (!brokenTxns.isEmpty()) {
+                            final List<Txn> txnsForId = brokenTxns.get(idAsLong);
+                            if (txnsForId != null) {
+                                    // Remove from brokenTxns
+                                    synchronized (txnsForId){
+                                        txnsForId.removeAll(fixed);
+                                        // can we get rid of txnsForId and brokenTxns?
+                                        if (!txnsForId.isEmpty()){
+                                            // Make sure we only remove txnsForId and not
some other collection.
+                                            brokenTxns.remove(idAsLong, txnsForId);
+                                        }
+                                    }
+                                    // Put in txns
+                                    for (Iterator i = fixed.iterator(); i.hasNext();) {
+                                            put((Txn) i.next());
+                                    }
+                            } else {
+                                    /*
+                                     * if the list for this id is gone the fixed Txns must
+                                     * have already been moved by someone else
+                                     */
+                            }
+                    } else {
+                            /*
+                             * if brokenTxns is gone the fixed Txns must have already
+                             * been moved by someone else
+                             */
+                    }
+			
 		}
 
 		// Did we run out of candidates, or did we find a match?
@@ -345,33 +344,30 @@ class TxnTable {
 		final long internalID = OutriggerServerImpl.nextID();
 
 		// Atomic test and set
-		synchronized (this) {
-			Txn r = (Txn) txns.get(k);
-			if (r == null) {
-				// not in table, put a new one in and return it.
-				r = new Txn(tr, internalID);
-				txns.put(k, r);
-			}
-
-			return r;
-		}
+                Txn r = txns.get(k);
+                if (r == null) {
+                    // not in table, put a new one in and return it.
+                    r = new Txn(tr, internalID);
+                    Txn existed = txns.putIfAbsent(k, r);
+                    if (existed != null) r = existed;
+                }
+                return r;
 	}
 
 	/**
-	 * Used to put a formally broken <code>Txn</code> in the main table. Only
-	 * puts it in the table if it is not already their.
+	 * Used to put a formerly broken <code>Txn</code> in the main table. Only
+	 * puts it in the table if it is not already there.
 	 * 
 	 * @param txn
 	 *            the <code>Txn</code> being moved, should have been prepared
 	 */
 	private void put(Txn txn) {
 		final Key k = new Key(txn.getManager(), txn.getTransactionId(), true);
-		synchronized (this) {
-			final Txn r = (Txn) txns.get(k);
-			if (r == null) {
-				txns.put(k, txn);
-			}
-		}
+                final Txn r = txns.get(k);
+                if (r == null) {
+                        // Doesn't add the key if already in there.
+                        txns.putIfAbsent(k, txn);
+                }
 	}
 
 	/**
@@ -401,17 +397,33 @@ class TxnTable {
 
 		if (st == null) {
 			// txn is broken, put in brokenTxns
-			if (brokenTxns == null)
-				brokenTxns = new java.util.HashMap();
-
 			final Long id = Long.valueOf(txn.getTransactionId());
-			List txnsForId = (List) brokenTxns.get(id);
+			List<Txn> txnsForId = brokenTxns.get(id);
 			if (txnsForId == null) {
-				txnsForId = new java.util.LinkedList();
-				brokenTxns.put(id, txnsForId);
+                            txnsForId = new java.util.LinkedList<Txn>();
+                            txnsForId.add(txn); // Never add an empty collection.
+                            List<Txn> existed = brokenTxns.putIfAbsent(id, txnsForId);
+                            while (existed != null) {
+                                synchronized (existed){
+                                    // Never add to an empty collection, it may have been
removed.
+                                    // Try to replace it with our shiny new collection.
+                                    if (existed.isEmpty()){
+                                        boolean replaced = brokenTxns.replace(id, existed,
txnsForId);
+                                        if (!replaced){
+                                            //This means someone else has replaced it first.
+                                            existed = brokenTxns.putIfAbsent(id, txnsForId);
+                                            //Existed will be another instance, very low
probability of null.
+                                            //If it is another instance we're not sync'd
on it until next loop.
+                                        }
+                                    } else {
+                                        existed.add(txn);
+                                        existed = null;
+                                    }
+                                    
+                                }
+                            }        
 			}
-
-			txnsForId.add(txn);
+                        
 		} else {
 			// Txn is ok, put it in txns
 			final Key k = new Key(st.mgr, st.id, true);
@@ -430,8 +442,6 @@ class TxnTable {
 	 */
 	void remove(TransactionManager manager, long id) {
 		final Key k = new Key(manager, id, false);
-		synchronized (this) {
-			txns.remove(k);
-		}
+                txns.remove(k);
 	}
 }

Modified: river/jtsk/skunk/qa_refactor/trunk/src/net/jini/activation/ActivatableInvocationHandler.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/net/jini/activation/ActivatableInvocationHandler.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/net/jini/activation/ActivatableInvocationHandler.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/net/jini/activation/ActivatableInvocationHandler.java
Thu Mar  7 09:38:09 2013
@@ -226,15 +226,17 @@ public final class ActivatableInvocation
      * does not implement RemoteMethodControl.
      */
     private boolean hasConsistentConstraints() {
-	if (uproxy instanceof RemoteMethodControl) {
-	    MethodConstraints uproxyConstraints =
-		((RemoteMethodControl) uproxy).getConstraints();
-	    return (clientConstraints == null ?
-		    uproxyConstraints == null :
-		    clientConstraints.equals(uproxyConstraints));
-	} else {
-	    return true;
-	}
+        synchronized (this){
+            if (uproxy instanceof RemoteMethodControl) {
+                MethodConstraints uproxyConstraints =
+                    ((RemoteMethodControl) uproxy).getConstraints();
+                return (clientConstraints == null ?
+                        uproxyConstraints == null :
+                        clientConstraints.equals(uproxyConstraints));
+            } else {
+                return true;
+            }
+        }
     }
     
     /**
@@ -1080,7 +1082,10 @@ public final class ActivatableInvocation
 		throw new ActivateFailedException("unexpected activation id");
 	    }
 	    
-	    Remote newUproxy = handler.uproxy;
+	    Remote newUproxy = null;
+            synchronized (handler){
+                newUproxy = handler.uproxy;
+            }
 	    if (newUproxy == null) {
 		throw new ActivateFailedException("null underlying proxy");
 	    } else if (newUproxy instanceof RemoteMethodControl) {

Modified: river/jtsk/skunk/qa_refactor/trunk/src/net/jini/config/ConfigurationFile.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/net/jini/config/ConfigurationFile.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/net/jini/config/ConfigurationFile.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/net/jini/config/ConfigurationFile.java Thu Mar
 7 09:38:09 2013
@@ -746,8 +746,8 @@ public class ConfigurationFile extends A
 		    }
 		    evaluated = true;
 		}
+                return value;
 	    }
-	    return value;
 	}
     }
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/net/jini/jeri/BasicInvocationHandler.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/net/jini/jeri/BasicInvocationHandler.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/net/jini/jeri/BasicInvocationHandler.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/net/jini/jeri/BasicInvocationHandler.java Thu Mar
 7 09:38:09 2013
@@ -643,10 +643,10 @@ public class BasicInvocationHandler
 	}
 
 	OutboundRequestIterator iter = oe.newCall(constraints);
-	if (!iter.hasNext()) {
-	    throw new ConnectIOException("iterator produced no requests",
-		new IOException("iterator produced no requests"));
-	}
+            if (!iter.hasNext()) {
+                throw new ConnectIOException("iterator produced no requests",
+                    new IOException("iterator produced no requests"));
+            }
 
 	Failure failure = null;
 	do {

Modified: river/jtsk/skunk/qa_refactor/trunk/src/net/jini/jeri/ssl/SslServerEndpointImpl.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/net/jini/jeri/ssl/SslServerEndpointImpl.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/net/jini/jeri/ssl/SslServerEndpointImpl.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/net/jini/jeri/ssl/SslServerEndpointImpl.java Thu
Mar  7 09:38:09 2013
@@ -318,8 +318,8 @@ class SslServerEndpointImpl extends Util
 	    if (sslSocketFactory == null) {
 		sslInit();
 	    }
+            return sslSocketFactory;
 	}
-	return sslSocketFactory;
     }
 
     /** Returns the ServerAuthManager, calling sslInit if needed. */
@@ -328,8 +328,8 @@ class SslServerEndpointImpl extends Util
 	    if (authManager == null) {
 		sslInit();
 	    }
-	}
 	return authManager;
+	}
     }
 
     /** Returns a hash code value for this object. */

Modified: river/jtsk/skunk/qa_refactor/trunk/src/net/jini/security/AuthenticationPermission.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/net/jini/security/AuthenticationPermission.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/net/jini/security/AuthenticationPermission.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/net/jini/security/AuthenticationPermission.java
Thu Mar  7 09:38:09 2013
@@ -334,13 +334,16 @@ public final class AuthenticationPermiss
 	    if (!nm.startsWith("\"")) {
 		throw new IllegalArgumentException("name must be in quotes");
 	    }
-	    while (!nm.endsWith("\"")) {
+            StringBuilder sb = new StringBuilder(120);
+            sb.append(nm);
+	    while (!sb.substring(sb.length()-1).equals("\"")) {
 		if (!st.hasMoreTokens()) {
 		    throw new IllegalArgumentException(
 						   "name must be in quotes");
 		}
-		nm = nm + st.nextToken();
+		sb.append(st.nextToken());
 	    }
+            nm = sb.toString();
 	    if (nm.equals("\"*\"")) {
 		if (peer) {
 		    throw new IllegalArgumentException(

Modified: river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/io/Distributed.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/io/Distributed.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/io/Distributed.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/io/Distributed.java Thu Mar
 7 09:38:09 2013
@@ -20,7 +20,9 @@ package org.apache.river.api.io;
  * Distributed objects are immutable value objects with final fields that may
  * be freely replicated.
  * Distributed objects are not serialized, instead they are only created using a 
- * public constructor, public static factory method or builder object.
+ * public constructor, public static factory method or builder object making them
+ * more suitable for security, validating class invariants and concurrent code
+ * that relies on immutability.
  * <p>
  * Distributed objects are free to evolve and may have completely different 
  * classes or be completely unequal after distribution to separate nodes, 
@@ -29,12 +31,16 @@ package org.apache.river.api.io;
  * <p>
  * Distributed objects have no version, instead SerialReflectionFactory contains all 
  * information required to distribute and recreate any Distributed Object using
- * reflection.
+ * reflection.  For this reason, Distributed objects cannot be used as Entry
+ * objects, which is dependant on published serial form.  It may be possible
+ * in a later release to use Distributed objects as fields in Entry objects, this
+ * is not supported presently.
  * <p>
  * Distributed objects are value objects from a domain driven design perspective.
  * <p>
  * Although final is not enforced, all fields must be final, safe
- * construction must be honored, distributed objects will be exposed to multiple
+ * construction must be honored  the this reference must not be allowed to 
+ * escape during construction, distributed objects will be exposed to multiple
  * threads on multiple nodes, without synchronization or transactions.
  * <p>
  * Do not use Distributed if you don't intend to honor this contract, use

Modified: river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PolicyUtils.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PolicyUtils.java?rev=1453746&r1=1453745&r2=1453746&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PolicyUtils.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/org/apache/river/api/security/PolicyUtils.java
Thu Mar  7 09:38:09 2013
@@ -240,7 +240,7 @@ import org.apache.river.impl.net.UriStri
             path = path.replace(File.separatorChar, '/');
             path = path.toUpperCase();
         }
-        if (!path.startsWith("/")) { //$NON-NLS-1$
+        if ( path != null && !path.startsWith("/")) { //$NON-NLS-1$
             return new URI("file", null, //$NON-NLS-1$
                     new StringBuilder(path.length() + 1).append('/')
                             .append(path).toString(), null, null);



Mime
View raw message