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/08 08:17:14 UTC
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/
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