HBASE-14784 Port conflict is not resolved in HBaseTestingUtility.randomFreePort() (Youngjoon Kim)

hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java

@@ -203,9 +203,6 @@ public class HBaseTestingUtility extends HBaseCommonTestingUtility {
   /** Filesystem URI used for map-reduce mini-cluster setup */
   private static String FS_URI;
 
-  /** A set of ports that have been claimed using {@link #randomFreePort()}. */
-  private static final Set<Integer> takenRandomPorts = new HashSet<Integer>();
-
   /** Compression algorithms to use in parameterized JUnit 4 tests */
   public static final List<Object[]> COMPRESSION_ALGORITHMS_PARAMETERIZED =
     Arrays.asList(new Object[][] {
@@ -3429,41 +3426,77 @@ public class HBaseTestingUtility extends HBaseCommonTestingUtility {
     return table;
   }
 
-  private static final int MIN_RANDOM_PORT = 0xc000;
-  private static final int MAX_RANDOM_PORT = 0xfffe;
   private static Random random = new Random();
 
-  /**
-   * Returns a random port. These ports cannot be registered with IANA and are
-   * intended for dynamic allocation (see http://bit.ly/dynports).
-   */
-  public static int randomPort() {
-    return MIN_RANDOM_PORT
-        + random.nextInt(MAX_RANDOM_PORT - MIN_RANDOM_PORT);
+  private static final PortAllocator portAllocator = new PortAllocator(random);
+
+  public static int randomFreePort() {
+    return portAllocator.randomFreePort();
   }
 
-  /**
-   * Returns a random free port and marks that port as taken. Not thread-safe. Expected to be
-   * called from single-threaded test setup code/
-   */
-  public static int randomFreePort() {
-    int port = 0;
-    do {
-      port = randomPort();
-      if (takenRandomPorts.contains(port)) {
-        port = 0;
-        continue;
-      }
-      takenRandomPorts.add(port);
+  static class PortAllocator {
+    private static final int MIN_RANDOM_PORT = 0xc000;
+    private static final int MAX_RANDOM_PORT = 0xfffe;
 
-      try {
-        ServerSocket sock = new ServerSocket(port);
-        sock.close();
-      } catch (IOException ex) {
-        port = 0;
-      }
-    } while (port == 0);
-    return port;
+    /** A set of ports that have been claimed using {@link #randomFreePort()}. */
+    private final Set<Integer> takenRandomPorts = new HashSet<Integer>();
+
+    private final Random random;
+    private final AvailablePortChecker portChecker;
+
+    public PortAllocator(Random random) {
+      this.random = random;
+      this.portChecker = new AvailablePortChecker() {
+        public boolean available(int port) {
+          try {
+            ServerSocket sock = new ServerSocket(port);
+            sock.close();
+            return true;
+          } catch (IOException ex) {
+            return false;
+          }
+        }
+      };
+    }
+
+    public PortAllocator(Random random, AvailablePortChecker portChecker) {
+      this.random = random;
+      this.portChecker = portChecker;
+    }
+
+    /**
+     * Returns a random free port and marks that port as taken. Not thread-safe. Expected to be
+     * called from single-threaded test setup code/
+     */
+    public int randomFreePort() {
+      int port = 0;
+      do {
+        port = randomPort();
+        if (takenRandomPorts.contains(port)) {
+          port = 0;
+          continue;
+        }
+        takenRandomPorts.add(port);
+
+        if (!portChecker.available(port)) {
+          port = 0;
+        }
+      } while (port == 0);
+      return port;
+    }
+
+    /**
+     * Returns a random port. These ports cannot be registered with IANA and are
+     * intended for dynamic allocation (see http://bit.ly/dynports).
+     */
+    private int randomPort() {
+      return MIN_RANDOM_PORT
+          + random.nextInt(MAX_RANDOM_PORT - MIN_RANDOM_PORT);
+    }
+
+    interface AvailablePortChecker {
+      boolean available(int port);
+    }
   }
 
 

hbase-server/src/test/java/org/apache/hadoop/hbase/TestHBaseTestingUtility.java

@@ -21,6 +21,7 @@ package org.apache.hadoop.hbase;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 
 import org.apache.commons.logging.Log;
@@ -40,9 +41,13 @@ import org.apache.hadoop.hdfs.MiniDFSCluster;
 import org.apache.hadoop.hbase.http.ssl.KeyStoreTestUtil;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.mockito.Mockito;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 
 import java.io.File;
 import java.util.List;
+import java.util.Random;
 
 /**
  * Test our testing utility class
@@ -385,5 +390,32 @@ public class TestHBaseTestingUtility {
     assertTrue(hbt.cleanupTestDir());
   }
 
-}
+  @Test public void testResolvePortConflict() throws Exception {
+    // raises port conflict between 1st call and 2nd call of randomPort() by mocking Random object
+    Random random = Mockito.mock(Random.class);
+    Mockito.when(random.nextInt(Mockito.any(Integer.class)))
+      .thenAnswer(new Answer<Integer>() {
+        int[] numbers = { 1, 1, 2 };
+        int count = 0;
 
+        @Override
+        public Integer answer(InvocationOnMock invocation) {
+          int ret = numbers[count];
+          count++;
+          return ret;
+        }
+      });
+
+    HBaseTestingUtility.PortAllocator.AvailablePortChecker portChecker =
+      Mockito.mock(HBaseTestingUtility.PortAllocator.AvailablePortChecker.class);
+    Mockito.when(portChecker.available(Mockito.any(Integer.class))).thenReturn(true);
+
+    HBaseTestingUtility.PortAllocator portAllocator =
+      new HBaseTestingUtility.PortAllocator(random, portChecker);
+
+    int port1 = portAllocator.randomFreePort();
+    int port2 = portAllocator.randomFreePort();
+    assertNotEquals(port1, port2);
+    Mockito.verify(random, Mockito.times(3)).nextInt(Mockito.any(Integer.class));
+  }
+}