NO-JIRA Adding checking for leaking server socket

artemis-commons/src/test/java/org/apache/activemq/artemis/utils/PortCheckRule.java

@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.utils;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.net.ServerSocket;
+
+import org.junit.Assert;
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
+
+/**
+ * This will make sure any properties changed through tests are cleaned up between tests.
+ */
+public class PortCheckRule extends TestWatcher {
+
+   final int[] port;
+
+   public PortCheckRule(int... port) {
+      this.port = port;
+   }
+
+   @Override
+   protected void starting(Description description) {
+      for (int p : port) {
+         if (!checkAvailable(p)) {
+            Assert.fail("a previous test is using port " + p + " on " + description);
+         }
+      }
+   }
+
+   @Override
+   protected void finished(Description description) {
+      for (int p : port) {
+         if (!checkAvailable(p)) {
+            Assert.fail(description + " has left a server socket open on port " + p);
+         }
+      }
+   }
+
+   public static boolean checkAvailable(int port) {
+      ServerSocket s = null;
+      try {
+         s = new ServerSocket();
+         s.bind(new InetSocketAddress("localhost", 61616));
+         return true;
+      } catch (IOException e) {
+         e.printStackTrace();
+         return false;
+      } finally {
+         try {
+            s.close();
+         } catch (Throwable ignored) {
+         }
+      }
+   }
+
+}

artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/ActiveMQTestBase.java

@@ -36,7 +36,6 @@ import java.io.ObjectOutputStream;
 import java.io.OutputStream;
 import java.lang.management.ManagementFactory;
 import java.lang.ref.WeakReference;
-import java.net.ServerSocket;
 import java.sql.Connection;
 import java.sql.Driver;
 import java.sql.DriverManager;
@@ -139,6 +138,7 @@ import org.apache.activemq.artemis.utils.ActiveMQThreadFactory;
 import org.apache.activemq.artemis.utils.CleanupSystemPropertiesRule;
 import org.apache.activemq.artemis.utils.Env;
 import org.apache.activemq.artemis.utils.FileUtil;
+import org.apache.activemq.artemis.utils.PortCheckRule;
 import org.apache.activemq.artemis.utils.RandomUtil;
 import org.apache.activemq.artemis.utils.ThreadDumpUtil;
 import org.apache.activemq.artemis.utils.ThreadLeakCheckRule;
@@ -148,6 +148,7 @@ import org.jboss.logging.Logger;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.rules.TemporaryFolder;
 import org.junit.rules.TestName;
@@ -167,13 +168,16 @@ public abstract class ActiveMQTestBase extends Assert {
    private static final Logger logger = Logger.getLogger(ActiveMQTestBase.class);
 
    /** This will make sure threads are not leaking between tests */
-   @Rule
-   public ThreadLeakCheckRule leakCheckRule = new ThreadLeakCheckRule();
+   @ClassRule
+   public static ThreadLeakCheckRule leakCheckRule = new ThreadLeakCheckRule();
 
    /** This will cleanup any system property changed inside tests */
    @Rule
    public CleanupSystemPropertiesRule propertiesRule = new CleanupSystemPropertiesRule();
 
+   @ClassRule
+   public static PortCheckRule portCheckRule = new PortCheckRule(61616);
+
    public static final String TARGET_TMP = "./target/tmp";
    public static final String INVM_ACCEPTOR_FACTORY = InVMAcceptorFactory.class.getCanonicalName();
    public static final String INVM_CONNECTOR_FACTORY = InVMConnectorFactory.class.getCanonicalName();
@@ -365,8 +369,6 @@ public abstract class ActiveMQTestBase extends Assert {
       OperationContextImpl.clearContext();
 
       InVMRegistry.instance.clear();
-
-      // checkFreePort(TransportConstants.DEFAULT_PORT);
    }
 
    public static void assertEqualsByteArrays(final byte[] expected, final byte[] actual) {
@@ -794,24 +796,6 @@ public abstract class ActiveMQTestBase extends Assert {
       return connectors;
    }
 
-   protected static final void checkFreePort(final int... ports) {
-      for (int port : ports) {
-         ServerSocket ssocket = null;
-         try {
-            ssocket = new ServerSocket(port);
-         } catch (Exception e) {
-            throw new IllegalStateException("port " + port + " is bound", e);
-         } finally {
-            if (ssocket != null) {
-               try {
-                  ssocket.close();
-               } catch (IOException e) {
-               }
-            }
-         }
-      }
-   }
-
    /**
     * @return the testDir
     */

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/cluster/distribution/ClusterTestBase.java

@@ -86,9 +86,11 @@ import org.apache.activemq.artemis.core.server.impl.AddressInfo;
 import org.apache.activemq.artemis.core.server.impl.InVMNodeManager;
 import org.apache.activemq.artemis.tests.integration.IntegrationTestLogger;
 import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
+import org.apache.activemq.artemis.utils.PortCheckRule;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.ClassRule;
 
 public abstract class ClusterTestBase extends ActiveMQTestBase {
 
@@ -96,6 +98,9 @@ public abstract class ClusterTestBase extends ActiveMQTestBase {
 
    private static final int[] PORTS = {TransportConstants.DEFAULT_PORT, TransportConstants.DEFAULT_PORT + 1, TransportConstants.DEFAULT_PORT + 2, TransportConstants.DEFAULT_PORT + 3, TransportConstants.DEFAULT_PORT + 4, TransportConstants.DEFAULT_PORT + 5, TransportConstants.DEFAULT_PORT + 6, TransportConstants.DEFAULT_PORT + 7, TransportConstants.DEFAULT_PORT + 8, TransportConstants.DEFAULT_PORT + 9,};
 
+   @ClassRule
+   public static PortCheckRule rule = new PortCheckRule(PORTS);
+
    protected int getLargeMessageSize() {
       return 500;
    }
@@ -131,8 +136,6 @@ public abstract class ClusterTestBase extends ActiveMQTestBase {
 
       forceGC();
 
-      ActiveMQTestBase.checkFreePort(ClusterTestBase.PORTS);
-
       consumers = new ConsumerHolder[ClusterTestBase.MAX_CONSUMERS];
 
       servers = new ActiveMQServer[ClusterTestBase.MAX_SERVERS];
@@ -175,8 +178,6 @@ public abstract class ClusterTestBase extends ActiveMQTestBase {
 
       super.tearDown();
 
-      ActiveMQTestBase.checkFreePort(ClusterTestBase.PORTS);
-
    }
 
    // Private -------------------------------------------------------------------------------------------------------

tests/jms-tests/pom.xml

@@ -39,6 +39,13 @@
          <scope>test</scope>
          <type>test-jar</type>
       </dependency>
+      <dependency>
+         <groupId>org.apache.activemq</groupId>
+         <artifactId>artemis-commons</artifactId>
+         <version>${project.version}</version>
+         <scope>test</scope>
+         <type>test-jar</type>
+      </dependency>
       <dependency>
          <groupId>org.apache.activemq</groupId>
          <artifactId>artemis-core-client</artifactId>

tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/remoting/impl/netty/NettyAcceptorTest.java

@@ -34,6 +34,7 @@ import org.apache.activemq.artemis.spi.core.remoting.Connection;
 import org.apache.activemq.artemis.spi.core.remoting.ServerConnectionLifeCycleListener;
 import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
 import org.apache.activemq.artemis.utils.ActiveMQThreadFactory;
+import org.apache.activemq.artemis.utils.PortCheckRule;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -48,24 +49,18 @@ public class NettyAcceptorTest extends ActiveMQTestBase {
    @Before
    public void setUp() throws Exception {
       super.setUp();
-
-      ActiveMQTestBase.checkFreePort(TransportConstants.DEFAULT_PORT);
    }
 
    @Override
    @After
    public void tearDown() throws Exception {
-      try {
-         ActiveMQTestBase.checkFreePort(TransportConstants.DEFAULT_PORT);
-      } finally {
+      if (pool3 != null)
+         pool3.shutdown();
 
-         if (pool3 != null)
-            pool3.shutdown();
+      if (pool2 != null)
+         pool2.shutdownNow();
 
-         if (pool2 != null)
-            pool2.shutdownNow();
-         super.tearDown();
-      }
+      super.tearDown();
    }
 
    @Test
@@ -107,13 +102,13 @@ public class NettyAcceptorTest extends ActiveMQTestBase {
       Assert.assertTrue(acceptor.isStarted());
       acceptor.stop();
       Assert.assertFalse(acceptor.isStarted());
-      ActiveMQTestBase.checkFreePort(TransportConstants.DEFAULT_PORT);
+      Assert.assertTrue(PortCheckRule.checkAvailable(TransportConstants.DEFAULT_PORT));
 
       acceptor.start();
       Assert.assertTrue(acceptor.isStarted());
       acceptor.stop();
       Assert.assertFalse(acceptor.isStarted());
-      ActiveMQTestBase.checkFreePort(TransportConstants.DEFAULT_PORT);
+      Assert.assertTrue(PortCheckRule.checkAvailable(TransportConstants.DEFAULT_PORT));
    }
 
 }