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);
