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); }
