river-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From peter_firmst...@apache.org
Subject svn commit: r1454257 - in /river/jtsk/skunk/qa_refactor/trunk/src: com/sun/jini/landlord/ com/sun/jini/thread/ net/jini/discovery/ net/jini/lookup/
Date Fri, 08 Mar 2013 07:17:14 GMT
Author: peter_firmstone
Date: Fri Mar  8 07:17:13 2013
New Revision: 1454257

URL: http://svn.apache.org/r1454257
Log:
Fixed visibility race bug in LandlordLease and friends.

Fixed more synchronisation bugs identified by FindBugs.

Modified:
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/landlord/ConstrainableLandlordLease.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/landlord/LandlordLease.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/InProgress.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/RetryTask.java
    river/jtsk/skunk/qa_refactor/trunk/src/net/jini/discovery/LookupDiscoveryManager.java
    river/jtsk/skunk/qa_refactor/trunk/src/net/jini/discovery/LookupLocatorDiscovery.java
    river/jtsk/skunk/qa_refactor/trunk/src/net/jini/lookup/JoinManager.java
    river/jtsk/skunk/qa_refactor/trunk/src/net/jini/lookup/ServiceDiscoveryManager.java

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/landlord/ConstrainableLandlordLease.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/landlord/ConstrainableLandlordLease.java?rev=1454257&r1=1454256&r2=1454257&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/landlord/ConstrainableLandlordLease.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/landlord/ConstrainableLandlordLease.java
Fri Mar  8 07:17:13 2013
@@ -236,8 +236,10 @@ final public class ConstrainableLandlord
 
     // doc inherited from super
     public RemoteMethodControl setConstraints(MethodConstraints constraints) {
-	return new ConstrainableLandlordLease(cookie(), landlord(), 
-	    landlordUuid(), expiration, constraints);
+        synchronized (this){
+            return new ConstrainableLandlordLease(cookie(), landlord(), 
+                landlordUuid(), expiration, constraints);
+        }
     }
 
     // doc inherited from super

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/landlord/LandlordLease.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/landlord/LandlordLease.java?rev=1454257&r1=1454256&r2=1454257&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/landlord/LandlordLease.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/landlord/LandlordLease.java Fri Mar
 8 07:17:13 2013
@@ -167,7 +167,9 @@ public class LandlordLease extends Abstr
 
     /** Set the expiration. */
     void setExpiration(long expiration) {
-	this.expiration = expiration;
+        synchronized (this){
+            this.expiration = expiration;
+        }
     }
 
     // inherit doc comment

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/InProgress.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/InProgress.java?rev=1454257&r1=1454256&r2=1454257&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/InProgress.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/InProgress.java Fri Mar  8
07:17:13 2013
@@ -168,7 +168,9 @@ public class InProgress {
      * Set the debug mode on or off.
      */
     public void debug(boolean debugOn) {
-	debug = debugOn;
+        synchronized (this){
+            debug = debugOn;
+        }
     }
 
     /**

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/RetryTask.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/RetryTask.java?rev=1454257&r1=1454256&r2=1454257&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/RetryTask.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/RetryTask.java Fri Mar  8 07:17:13
2013
@@ -166,8 +166,7 @@ public abstract class RetryTask implemen
      * not own the lock the result is undefined and could result in an
      * exception.
      */
-    public long retryTime() {
-	assert Thread.holdsLock(this): "thread does not hold lock";
+    public synchronized long retryTime() {
 	int index = (attempt < delays.length ? attempt : delays.length - 1); 
 	return delays[index] + System.currentTimeMillis();
     }

Modified: river/jtsk/skunk/qa_refactor/trunk/src/net/jini/discovery/LookupDiscoveryManager.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/net/jini/discovery/LookupDiscoveryManager.java?rev=1454257&r1=1454256&r2=1454257&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/net/jini/discovery/LookupDiscoveryManager.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/net/jini/discovery/LookupDiscoveryManager.java
Fri Mar  8 07:17:13 2013
@@ -386,18 +386,25 @@ public class LookupDiscoveryManager impl
          *  from the <code>LookupLocatorDiscovery</code>.
          */
 	public void discard() {
+            LookupDiscovery lookD = null;
+            LookupLocatorDiscovery locateD = null;
+            ServiceRegistrar regProxy = null;
 	    synchronized(this) {
 		bDiscarded = true;
+                regProxy = proxy;
+                if((from & FROM_GROUP) != 0) {
+                    lookD = lookupDisc;
+                } 
+                if((from & FROM_LOCATOR) != 0) {
+                    locateD = locatorDisc;
+                }//endif
 	    }//end sync
             /* Give group discovery priority to avoid race condition. When
              * appropriate, locator discard will eventually occur as a result
              * of the first group discard.
              */
-	    if((from & FROM_GROUP) != 0) {
-                lookupDisc.discard(proxy);
-	    } else if((from & FROM_LOCATOR) != 0) {
-                locatorDisc.discard(proxy);
-            }//endif
+            if (lookD != null) lookD.discard(regProxy);
+            if (locateD != null) locateD.discard(regProxy);
 	}//end discard
 
         /** Accessor method that returns the value of the <code>boolean</code>

Modified: river/jtsk/skunk/qa_refactor/trunk/src/net/jini/discovery/LookupLocatorDiscovery.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/net/jini/discovery/LookupLocatorDiscovery.java?rev=1454257&r1=1454256&r2=1454257&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/net/jini/discovery/LookupLocatorDiscovery.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/net/jini/discovery/LookupLocatorDiscovery.java
Fri Mar  8 07:17:13 2013
@@ -436,26 +436,32 @@ public class LookupLocatorDiscovery impl
          *  available for quite some time (if ever).
 	 */
 	public void calcNextTryTime() {
-	    nextTryTime = System.currentTimeMillis() + sleepTime[tryIndx];
-	    if(tryIndx < sleepTime.length-1)  tryIndx++;
+            synchronized (this){
+                nextTryTime = System.currentTimeMillis() + sleepTime[tryIndx];
+                if(tryIndx < sleepTime.length-1)  tryIndx++;
+            }
 	}//end calcNextTryTime
 
         /** This method gets called only from the public discard() method.
          *  The purpose of this method is to delay the next discovery attempt.
 	 */
 	public void delayNextTryTime()  {
-	    discarded = true;
-	    tryIndx = 2;
+            synchronized (this){
+                discarded = true;
+                tryIndx = 2;
+            }
 	}
 
         /** Initiates unicast discovery of the lookup service referenced 
          *  in this class.
          */
 	public boolean tryGetProxy() {
-	    if (proxy != null ) {
-		throw new IllegalArgumentException
-                                 ("LookupLocator has been discovered already");
-	    }
+            synchronized(this){
+                if (proxy != null ) {
+                    throw new IllegalArgumentException
+                                     ("LookupLocator has been discovered already");
+                }
+            }
 	    InvocationConstraints ic = InvocationConstraints.EMPTY;
 	    if (l instanceof RemoteMethodControl) {
 		MethodConstraints mc =

Modified: river/jtsk/skunk/qa_refactor/trunk/src/net/jini/lookup/JoinManager.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/net/jini/lookup/JoinManager.java?rev=1454257&r1=1454256&r2=1454257&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/net/jini/lookup/JoinManager.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/net/jini/lookup/JoinManager.java Fri Mar  8 07:17:13
2013
@@ -1507,7 +1507,7 @@ public class JoinManager {
      */
     private long renewalDuration = Lease.FOREVER;
     /** Flag that indicates if this join manager has been terminated. */
-    private boolean bTerminated = false;
+    private volatile boolean bTerminated = false;
     /* Preparer for the proxies to the lookup services that are discovered
      * and used by this utility.
      */

Modified: river/jtsk/skunk/qa_refactor/trunk/src/net/jini/lookup/ServiceDiscoveryManager.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/net/jini/lookup/ServiceDiscoveryManager.java?rev=1454257&r1=1454256&r2=1454257&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/net/jini/lookup/ServiceDiscoveryManager.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/net/jini/lookup/ServiceDiscoveryManager.java Fri
Mar  8 07:17:13 2013
@@ -2462,7 +2462,7 @@ public class ServiceDiscoveryManager {
     /* Contains all of the discovered lookup services (ServiceRegistrar). */
     private final ArrayList proxyRegSet = new ArrayList(1);
     /* Contains all of the DiscoveryListener's employed in lookup discovery. */
-    private final ArrayList listeners = new ArrayList(1);
+    private final ArrayList<DiscoveryListener> listeners = new ArrayList<DiscoveryListener>(1);
     /* Random number generator for use in lookup. */
     private final Random random = new Random();
     /* Contains all of the instances of LookupCache that are requested. */
@@ -2530,7 +2530,7 @@ public class ServiceDiscoveryManager {
 	public void discarded(DiscoveryEvent e) {
 	    ServiceRegistrar[] proxys = (ServiceRegistrar[])e.getRegistrars();
 	    ArrayList notifies;
-	    ArrayList drops = new ArrayList(1);
+	    ArrayList<ProxyReg> drops = new ArrayList<ProxyReg>(1);
 	    synchronized(proxyRegSet) {
 		for(int i=0; i<proxys.length; i++) {
 		    ProxyReg reg = findReg(proxys[i]);
@@ -2538,19 +2538,23 @@ public class ServiceDiscoveryManager {
 			proxyRegSet.remove(proxyRegSet.indexOf(reg));
 			drops.add(reg);
 		    } else {
-			throw new RuntimeException("discard error");
+                        //River-337
+                        logger.severe("discard error, proxy was null");
+			//throw new RuntimeException("discard error");
                     }//endif
 		}//end loop
 	    }//end sync(proxyRegSet)
-	    Iterator iter = drops.iterator();
+	    Iterator<ProxyReg> iter = drops.iterator();
 	    while(iter.hasNext()) {
-		dropProxy((ProxyReg)iter.next());
+		dropProxy(iter.next());
             }//end loop
-	    synchronized(listeners) {
-		if(listeners.isEmpty()) return;
-		notifies = (ArrayList)listeners.clone();
-	    }//end sync(listeners)
-	    listenerDropped(drops, notifies);
+            if (!drops.isEmpty()){
+                synchronized(listeners) {
+                    if(listeners.isEmpty()) return;
+                    notifies = (ArrayList)listeners.clone();
+                }//end sync(listeners)
+                listenerDropped(drops, notifies);
+            }
 	}//end DiscMgrListener.discarded
 
     }//end class ServiceDiscoveryManager.DiscMgrListener



Mime
View raw message