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/03/07 10:38:10 UTC
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...
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);