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
