river-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From peter_firmst...@apache.org
Subject svn commit: r1557513 - in /river/jtsk/skunk/qa_refactor/trunk: qa/src/com/sun/jini/qa/harness/ qa/src/com/sun/jini/test/share/ qa/src/com/sun/jini/test/spec/discoveryservice/event/ qa/src/com/sun/jini/test/spec/discoveryservice/lease/ qa/src/com/sun/ji...
Date Sun, 12 Jan 2014 10:51:26 GMT
Author: peter_firmstone
Date: Sun Jan 12 10:51:25 2014
New Revision: 1557513

URL: http://svn.apache.org/r1557513
Log:
Minor alteration to Reggie, removed use of timed collection cache of AddressTask, replaced
with a Set that ensures no duplicate AddressTask's run concurrently.

Addressed some minor bugs and improvement suggestions identified by FindBugs in the QA test
suite tests.

Modified:
    river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/ActivationSystemAdmin.java
    river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/MasterHarness.java
    river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/Pipe.java
    river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/RemoteServiceAdmin.java
    river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/share/LeaseUsesTestBase.java
    river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/share/OpCountingOwner.java
    river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/discoveryservice/event/SetLocatorsReplaceAll.java
    river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/discoveryservice/lease/LeaseExpiration.java
    river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/jeri/mux/util/TimedReceiveTask.java
    river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/locatordiscovery/DiscoveredStagger.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/reggie/RegistrarImpl.java
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/ThreadPool.java
    river/jtsk/skunk/qa_refactor/trunk/src/net/jini/lookup/ServiceDiscoveryManager.java

Modified: river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/ActivationSystemAdmin.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/ActivationSystemAdmin.java?rev=1557513&r1=1557512&r2=1557513&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/ActivationSystemAdmin.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/ActivationSystemAdmin.java
Sun Jan 12 10:51:25 2014
@@ -334,8 +334,11 @@ public class ActivationSystemAdmin 
 	    logger.log(Level.FINEST, 
 		       "activation death delayed " + delay + " seconds");
 	    try {
-		Thread.sleep(delay * 1000);
+                // Changed from sleep to wait because of synchronization
+                // TODO: call in loop and check condition.
+		wait(delay * 1000);
 	    } catch (InterruptedException ignore) {
+                Thread.currentThread().interrupt();
 	    }
 	}
 	actSystem.shutdown();

Modified: river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/MasterHarness.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/MasterHarness.java?rev=1557513&r1=1557512&r2=1557513&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/MasterHarness.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/MasterHarness.java Sun
Jan 12 10:51:25 2014
@@ -1069,7 +1069,7 @@ class MasterHarness {
      * detected, the input stream is absorbed by this filter and used
      * to construct a <code>TestResult</code> object.
      */
-    private class TestResultFilter implements Pipe.Filter {
+    private static class TestResultFilter implements Pipe.Filter {
 
 	byte[] inputBuffer = new byte[STATUS_TOKEN.length()];
 	byte[] statusBytes = STATUS_TOKEN.getBytes();

Modified: river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/Pipe.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/Pipe.java?rev=1557513&r1=1557512&r2=1557513&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/Pipe.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/Pipe.java Sun Jan 12
10:51:25 2014
@@ -228,7 +228,7 @@ class Pipe implements Runnable {
     }
 
     /** A default implementation of the <code>Filter</code> interface */
-    private class NullFilter implements Filter {
+    private static class NullFilter implements Filter {
 	public byte[] filterInput(byte b) {
 	    return new byte[]{b};
 	}

Modified: river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/RemoteServiceAdmin.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/RemoteServiceAdmin.java?rev=1557513&r1=1557512&r2=1557513&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/RemoteServiceAdmin.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/qa/harness/RemoteServiceAdmin.java
Sun Jan 12 10:51:25 2014
@@ -149,7 +149,10 @@ class RemoteServiceAdmin extends Abstrac
      * @throws TestException of the call to the slave fails.
      */
     public boolean killVM() throws TestException {
-	SlaveRequest request = new KillVMRequest(serviceRef);
+	SlaveRequest request = null;
+        synchronized (this){
+            request = new KillVMRequest(serviceRef);
+        }
 	Boolean b = (Boolean) SlaveTest.call(hostname, request);
 	return b.booleanValue();
     }
@@ -180,8 +183,10 @@ class RemoteServiceAdmin extends Abstrac
      * @return the result of the accessor call
      */
     private Object callAccessor(String accessorName) {
-	SlaveRequest request = new AdminAccessorRequest(accessorName, 
-							 serviceRef);
+	SlaveRequest request = null;
+        synchronized (this){
+            request = new AdminAccessorRequest(accessorName, serviceRef);
+        }
 	try {
 	    return SlaveTest.call(hostname, request);
 	} catch (Exception e) {

Modified: river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/share/LeaseUsesTestBase.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/share/LeaseUsesTestBase.java?rev=1557513&r1=1557512&r2=1557513&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/share/LeaseUsesTestBase.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/share/LeaseUsesTestBase.java
Sun Jan 12 10:51:25 2014
@@ -197,7 +197,9 @@ public abstract class LeaseUsesTestBase 
 
 		    if (!isAcceptable(lease))
 			throw new TestException("Renewed lease had an improper duration");
-		    renewals--;
+                    synchronized (this){
+                        renewals--;
+                    }
 		} else if (renewals == 0 && cancel) {
 		    cancel();
 		    // If we are here the cancel worked, need to break

Modified: river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/share/OpCountingOwner.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/share/OpCountingOwner.java?rev=1557513&r1=1557512&r2=1557513&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/share/OpCountingOwner.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/share/OpCountingOwner.java
Sun Jan 12 10:51:25 2014
@@ -101,7 +101,7 @@ public class OpCountingOwner extends Bas
      */
     public void batchCancel()  {
 	++batchCancelCalls;
-	batchCancel();
+	super.batchCancel(); // Changed to call super on 12th Jan 2014 to avoid infinite recursion
     }
 
     /**

Modified: river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/discoveryservice/event/SetLocatorsReplaceAll.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/discoveryservice/event/SetLocatorsReplaceAll.java?rev=1557513&r1=1557512&r2=1557513&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/discoveryservice/event/SetLocatorsReplaceAll.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/discoveryservice/event/SetLocatorsReplaceAll.java
Sun Jan 12 10:51:25 2014
@@ -76,9 +76,11 @@ import java.util.Set;
  */
 public class SetLocatorsReplaceAll extends SetLocatorsReplaceSome {
 
-    protected volatile LookupLocator[] newLocatorsToDiscover = new LookupLocator[0];
-    protected volatile Map locatorsMap = new HashMap(1);
-    protected volatile HashSet proxiesReplaced = new HashSet(11);
+    // Two fields weren't used at all, the other, commented out below,
+    // obscured a superclass field that was used during the test run.
+    // the field below was only ever set and never used, which
+    // appeared to be a mistake. - P. Firmstone 12th Jan 2014.
+//    protected volatile Map locatorsMap = new HashMap(1);
 
     /** Performs actions necessary to prepare for execution of the 
      *  current test (refer to the description of this method in the

Modified: river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/discoveryservice/lease/LeaseExpiration.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/discoveryservice/lease/LeaseExpiration.java?rev=1557513&r1=1557512&r2=1557513&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/discoveryservice/lease/LeaseExpiration.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/discoveryservice/lease/LeaseExpiration.java
Sun Jan 12 10:51:25 2014
@@ -98,7 +98,7 @@ public class LeaseExpiration extends Abs
 						 ServerProxyTrust,
 						 Serializable 
     {
-        private final Exporter exporter;
+        private Exporter exporter;
         private Object proxy;
         
         public ServiceEventListener() throws RemoteException {
@@ -163,7 +163,7 @@ public class LeaseExpiration extends Abs
     /** Convenience class for monitoring the lease with the lookup discovery
      *  service.
      */
-    public class LRMListener implements LeaseListener {
+    public static class LRMListener implements LeaseListener {
         public LRMListener() {
             super();
         }

Modified: river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/jeri/mux/util/TimedReceiveTask.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/jeri/mux/util/TimedReceiveTask.java?rev=1557513&r1=1557512&r2=1557513&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/jeri/mux/util/TimedReceiveTask.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/jeri/mux/util/TimedReceiveTask.java
Sun Jan 12 10:51:25 2014
@@ -23,6 +23,8 @@ import java.io.InputStream;
 
 /**
  * Utility class to receive a message within a given period of time.
+ * 
+ * TODO: This class appears to be dead code, it's not directly used by other classes.
  */
 public class TimedReceiveTask {
 

Modified: river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/locatordiscovery/DiscoveredStagger.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/locatordiscovery/DiscoveredStagger.java?rev=1557513&r1=1557512&r2=1557513&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/locatordiscovery/DiscoveredStagger.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/qa/src/com/sun/jini/test/spec/locatordiscovery/DiscoveredStagger.java
Sun Jan 12 10:51:25 2014
@@ -23,6 +23,7 @@ import com.sun.jini.qa.harness.Test;
 import com.sun.jini.qa.harness.QAConfig;
 import com.sun.jini.qa.harness.TestException;
 import com.sun.jini.test.share.LookupServices;
+import java.util.List;
 import net.jini.core.discovery.LookupLocator;
 
 /**
@@ -102,14 +103,15 @@ public class DiscoveredStagger extends A
             lookupsThread = lookups.staggeredStartThread(next);
             /* Re-configure LookupLocatorDiscovery to discover given locators*/
             logger.log(Level.FINE, "change LookupLocatorDiscovery to discover -- ");
+            List<LocatorGroupsPair> allLookupsToStart = getAllLookupsToStart();
             LookupLocator[] locatorsToDiscover
-                                          = toLocatorArray(getAllLookupsToStart());
+                                          = toLocatorArray(allLookupsToStart);
             for(int i=0;i<locatorsToDiscover.length;i++) {
                 logger.log(Level.FINE, "    "+locatorsToDiscover[i]);
             }//end loop
             locatorDiscovery.setLocators(locatorsToDiscover);
             /* Add the given listener to the LookupLocatorDiscovery utility */
-            mainListener.setLookupsToDiscover(getAllLookupsToStart());
+            mainListener.setLookupsToDiscover(allLookupsToStart);
             locatorDiscovery.addDiscoveryListener(mainListener);
             /* Start remaining lookup services in a time-staggered fashion */
             lookupsThread.start();

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/reggie/RegistrarImpl.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/reggie/RegistrarImpl.java?rev=1557513&r1=1557512&r2=1557513&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/reggie/RegistrarImpl.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/reggie/RegistrarImpl.java Sun Jan
12 10:51:25 2014
@@ -2087,31 +2087,21 @@ class RegistrarImpl implements Registrar
 
     /** Task for decoding multicast request packets. */
     private static final class DecodeRequestTask implements Runnable {
-        /* Keeps a record of recent AddressTasksto avoid DOS attacks. 
-         * 
-         * Recommended period between discovery multicast requests is 5 sec.
-         */
-        private static final Set<AddressTask> executedTasks;
-        static {
-            Comparator<Referrer<AddressTask>> comparator = RC.comparator(new
AddressTaskComparator());
-            executedTasks =
-                RC.set(new ConcurrentSkipListSet<Referrer<AddressTask>>(
-                        comparator), 
-                        Ref.TIME,
-                        2000L);
-        }
 	/** The multicast packet to decode */
 	private final DatagramPacket datagram;
 	/** The decoder for parsing the packet */
 	private final Discovery decoder;
         private final RegistrarImpl reggie;
+        /* Ensures that no duplicate AddressTask is running */
+        private final Set<AddressTask> runningTasks;
 
 	public DecodeRequestTask(
-                DatagramPacket datagram, Discovery decoder, RegistrarImpl reggie) 
+                DatagramPacket datagram, Discovery decoder, RegistrarImpl reggie, Set<AddressTask>
runningTasks) 
         {
 	    this.datagram = datagram;
 	    this.decoder = decoder;
             this.reggie = reggie;
+            this.runningTasks = runningTasks;
 	}
 
 	/**
@@ -2169,19 +2159,17 @@ class RegistrarImpl implements Registrar
 		}
 		AddressTask task = 
                         new AddressTask(req.getHost(), req.getPort(), reggie);
-                if (executedTasks.add(task)) task.run();
+                if (runningTasks.add(task)) {
+                    try {
+                        task.run();
+                    } finally {
+                        runningTasks.remove(task);
+                    }
+                }
 	    }
 	}
     }
 
-    private static class AddressTaskComparator implements Comparator<AddressTask>{
-
-        @Override
-        public int compare(AddressTask o1, AddressTask o2) {
-            return o1.compareTo(o2);
-        }
-        
-    }
     /** Address for unicast discovery response. */
     private static final class AddressTask implements Runnable, Comparable<AddressTask>
{
 
@@ -2539,6 +2527,8 @@ class RegistrarImpl implements Registrar
 	/** Interfaces for which configuration failed */
 	private final List<NetworkInterface> failedInterfaces;
         
+        private final Set<AddressTask> runningTasks;
+        
         private volatile boolean interrupted = false;
 
 	/**
@@ -2546,6 +2536,7 @@ class RegistrarImpl implements Registrar
 	 * rather than in run, so that we get any exception up front.
 	 */
 	public Multicast(RegistrarImpl reggie) throws IOException {
+            this.runningTasks = new ConcurrentSkipListSet<AddressTask>();
             this.reggie = reggie;
             List<NetworkInterface> failedInterfaces = new ArrayList<NetworkInterface>();
 	    if (reggie.multicastInterfaces != null && reggie.multicastInterfaces.length
== 0)
@@ -2641,7 +2632,8 @@ class RegistrarImpl implements Registrar
                             new DecodeRequestTask(
                                     dgram, 
                                     reggie.getDiscovery(pv), 
-                                    reggie
+                                    reggie,
+                                    runningTasks
                             )
                     );
 

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/ThreadPool.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/ThreadPool.java?rev=1557513&r1=1557512&r2=1557513&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/ThreadPool.java (original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/thread/ThreadPool.java Sun Jan 12
10:51:25 2014
@@ -20,15 +20,10 @@ package com.sun.jini.thread;
 
 import com.sun.jini.action.GetLongAction;
 import java.security.AccessController;
-import java.util.Queue;
 import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.SynchronousQueue;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.locks.Condition;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -117,7 +112,6 @@ final class ThreadPool implements Execut
     private final AtomicInteger threadCount;
     private final AtomicInteger waitingThreads;
     private final int delayFactor;
-    private static final int numberOfCores = Runtime.getRuntime().availableProcessors();
     
     ThreadPool(ThreadGroup threadGroup){
         this(threadGroup, 10);

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=1557513&r1=1557512&r2=1557513&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 Sun
Jan 12 10:51:25 2014
@@ -3176,7 +3176,7 @@ public class ServiceDiscoveryManager {
                     duration = cache.getLeaseDuration();
                 }//end loop
             }//end sync(cacheListener)
-            return sm;
+            return null; // Make it clear we're returning null.
         } finally {
             if(cache != null) terminator.terminate(cache);
         }



Mime
View raw message