QPID-4062 : [Java Tests] Java system tests sometimes fail due to JMX port already initialised (Applied patch from Philip Harvey

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@1349443 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 59 insertions, 13 deletions

diff --git a/java/systests/src/main/java/org/apache/qpid/test/utils/InternalBrokerHolder.java b/java/systests/src/main/java/org/apache/qpid/test/utils/InternalBrokerHolder.java
index adda9ca3ec..003fabed20 100644
--- a/java/systests/src/main/java/org/apache/qpid/test/utils/InternalBrokerHolder.java
+++ b/java/systests/src/main/java/org/apache/qpid/test/utils/InternalBrokerHolder.java
@@ -20,6 +20,8 @@
  */
 package org.apache.qpid.test.utils;
 
+import java.util.Set;
+
 import org.apache.log4j.Logger;
 
 import org.apache.qpid.server.Broker;
@@ -31,7 +33,9 @@ public class InternalBrokerHolder implements BrokerHolder
     private final Broker _broker;
     private final String _workingDirectory;
 
-    public InternalBrokerHolder(final Broker broker, String workingDirectory)
+    private Set<Integer> _portsUsedByBroker;
+
+    public InternalBrokerHolder(final Broker broker, String workingDirectory, Set<Integer> portsUsedByBroker)
     {
         if(broker == null)
         {
@@ -40,6 +44,7 @@ public class InternalBrokerHolder implements BrokerHolder
 
         _broker = broker;
         _workingDirectory = workingDirectory;
+        _portsUsedByBroker = portsUsedByBroker;
     }
 
     @Override
@@ -53,7 +58,9 @@ public class InternalBrokerHolder implements BrokerHolder
         LOGGER.info("Shutting down Broker instance");
 
         _broker.shutdown();
-        
+
+        waitUntilPortsAreFree();
+
         LOGGER.info("Broker instance shutdown");
     }
 
@@ -62,7 +69,12 @@ public class InternalBrokerHolder implements BrokerHolder
     {
         // Can't kill a internal broker as we would also kill ourselves as we share the same JVM.
         shutdown();
-    }
 
+        waitUntilPortsAreFree();
+    }
 
+    private void waitUntilPortsAreFree()
+    {
+        new PortHelper().waitUntilPortsAreFree(_portsUsedByBroker);
+    }
 }
diff --git a/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java b/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
index 238d3d5229..3338e8b3b9 100644
--- a/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
+++ b/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
@@ -30,6 +30,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+
 import javax.jms.BytesMessage;
 import javax.jms.Connection;
 import javax.jms.Destination;
@@ -46,6 +47,7 @@ import javax.jms.TextMessage;
 import javax.jms.Topic;
 import javax.naming.InitialContext;
 import javax.naming.NamingException;
+
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.configuration.XMLConfiguration;
 import org.apache.commons.lang.StringUtils;
@@ -163,13 +165,13 @@ public class QpidBrokerTestCase extends QpidTestCase
     protected List<Connection> _connections = new ArrayList<Connection>();
     public static final String QUEUE = "queue";
     public static final String TOPIC = "topic";
-    
+
     /** Map to hold test defined environment properties */
     private Map<String, String> _env;
 
     /** Ensure our messages have some sort of size */
     protected static final int DEFAULT_MESSAGE_SIZE = 1024;
-    
+
     /** Size to create our message*/
     private int _messageSize = DEFAULT_MESSAGE_SIZE;
     /** Type of message*/
@@ -308,6 +310,24 @@ public class QpidBrokerTestCase extends QpidTestCase
     }
 
     /**
+     * The returned set of port numbers is only a guess because it assumes no ports have been overridden
+     * using system properties.
+     */
+    protected Set<Integer> guessAllPortsUsedByBroker(int mainPort)
+    {
+        Set<Integer> ports = new HashSet<Integer>();
+        int managementPort = getManagementPort(mainPort);
+        int connectorServerPort = managementPort + ServerConfiguration.JMXPORT_CONNECTORSERVER_OFFSET;
+
+        ports.add(mainPort);
+        ports.add(managementPort);
+        ports.add(connectorServerPort);
+        ports.add(DEFAULT_SSL_PORT);
+
+        return ports;
+    }
+
+    /**
      * Get the Port that is use by the current broker
      *
      * @return the current port
@@ -367,6 +387,8 @@ public class QpidBrokerTestCase extends QpidTestCase
             throw new IllegalStateException("There is already an existing broker running on port " + port);
         }
 
+        Set<Integer> portsUsedByBroker = guessAllPortsUsedByBroker(port);
+
         if (_brokerType.equals(BrokerType.INTERNAL) && !existingInternalBroker())
         {
             setConfigurationProperty(ServerConfiguration.MGMT_CUSTOM_REGISTRY_SOCKET, String.valueOf(false));
@@ -393,7 +415,7 @@ public class QpidBrokerTestCase extends QpidTestCase
             _logger.info("starting internal broker (same JVM)");
             broker.startup(options);
 
-            _brokers.put(port, new InternalBrokerHolder(broker, System.getProperty("QPID_WORK")));
+            _brokers.put(port, new InternalBrokerHolder(broker, System.getProperty("QPID_WORK"), portsUsedByBroker));
         }
         else if (!_brokerType.equals(BrokerType.EXTERNAL))
         {
@@ -500,14 +522,14 @@ public class QpidBrokerTestCase extends QpidTestCase
                 // this is expect if the broker started successfully
             }
 
-            _brokers.put(port, new SpawnedBrokerHolder(process, qpidWork));
+            _brokers.put(port, new SpawnedBrokerHolder(process, qpidWork, portsUsedByBroker));
         }
     }
 
     private void addExcludedPorts(int port, int sslPort, BrokerOptions options)
     {
         final String protocolExcludesList = getProtocolExcludesList(port, sslPort);
-        
+
         if (protocolExcludesList.equals(""))
         {
             return;
@@ -1026,7 +1048,7 @@ public class QpidBrokerTestCase extends QpidTestCase
     {
         return (AMQConnectionFactory) getInitialContext().lookup(factoryName);
     }
-    
+
     public Connection getConnection() throws JMSException, NamingException
     {
         return getConnection("guest", "guest");
@@ -1320,14 +1342,14 @@ public class QpidBrokerTestCase extends QpidTestCase
 
     /**
      * Reloads the broker security configuration using the ApplicationRegistry (InVM brokers) or the
-     * ConfigurationManagementMBean via the JMX interface (Standalone brokers, management must be 
+     * ConfigurationManagementMBean via the JMX interface (Standalone brokers, management must be
      * enabled before calling the method).
      */
     public void reloadBrokerSecurityConfig() throws Exception
     {
         JMXTestUtils jmxu = new JMXTestUtils(this);
         jmxu.open();
-        
+
         try
         {
             ConfigurationManagement configMBean = jmxu.getConfigurationManagement();
@@ -1337,7 +1359,7 @@ public class QpidBrokerTestCase extends QpidTestCase
         {
             jmxu.close();
         }
-        
+
         LogMonitor _monitor = new LogMonitor(_outputFile);
         assertTrue("The expected server security configuration reload did not occur",
                 _monitor.waitForMessage(ServerConfiguration.SECURITY_CONFIG_RELOADED, LOGMONITOR_TIMEOUT));
diff --git a/java/systests/src/main/java/org/apache/qpid/test/utils/SpawnedBrokerHolder.java b/java/systests/src/main/java/org/apache/qpid/test/utils/SpawnedBrokerHolder.java
index 787fc164d5..ecf90ba431 100644
--- a/java/systests/src/main/java/org/apache/qpid/test/utils/SpawnedBrokerHolder.java
+++ b/java/systests/src/main/java/org/apache/qpid/test/utils/SpawnedBrokerHolder.java
@@ -21,6 +21,7 @@
 package org.apache.qpid.test.utils;
 
 import java.io.IOException;
+import java.util.Set;
 
 import org.apache.log4j.Logger;
 
@@ -32,8 +33,9 @@ public class SpawnedBrokerHolder implements BrokerHolder
     private final Process _process;
     private final Integer _pid;
     private final String _workingDirectory;
+    private Set<Integer> _portsUsedByBroker;
 
-    public SpawnedBrokerHolder(final Process process, final String workingDirectory)
+    public SpawnedBrokerHolder(final Process process, final String workingDirectory, Set<Integer> portsUsedByBroker)
     {
         if(process == null)
         {
@@ -43,6 +45,7 @@ public class SpawnedBrokerHolder implements BrokerHolder
         _process = process;
         _pid = retrieveUnixPidIfPossible();
         _workingDirectory = workingDirectory;
+        _portsUsedByBroker = portsUsedByBroker;
     }
 
     @Override
@@ -57,6 +60,8 @@ public class SpawnedBrokerHolder implements BrokerHolder
         _process.destroy();
 
         reapChildProcess();
+
+        waitUntilPortsAreFree();
     }
 
     @Override
@@ -74,6 +79,8 @@ public class SpawnedBrokerHolder implements BrokerHolder
         }
 
         reapChildProcess();
+
+        waitUntilPortsAreFree();
     }
 
     private void sendSigkillForImmediateShutdown(Integer pid)
@@ -146,4 +153,9 @@ public class SpawnedBrokerHolder implements BrokerHolder
         }
     }
 
+    private void waitUntilPortsAreFree()
+    {
+        new PortHelper().waitUntilPortsAreFree(_portsUsedByBroker);
+    }
+
 }