Improvements to PortUtil

hapi-fhir-base/src/main/java/ca/uhn/fhir/util/PortUtil.java

@@ -20,7 +20,6 @@ package ca.uhn.fhir.util;
  * #L%
  */
 
-import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -28,53 +27,95 @@ import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.ServerSocket;
 import java.net.Socket;
+import java.util.ArrayList;
+import java.util.List;
 
 /**
- * Provides server ports
+ * Provides server ports that are free, in order for tests to use them
+ *
+ * This class is not designed for non-testing usage, as it holds on to server ports
+ * for a long time (potentially lots of them!)
  */
 @CoverageIgnore
 public class PortUtil {
+	public static final int SPACE_SIZE = 100;
 	private static final Logger ourLog = LoggerFactory.getLogger(PortUtil.class);
+	private static final PortUtil INSTANCE = new PortUtil();
+	private List<ServerSocket> myControlSockets = new ArrayList<>();
+	private Integer myCurrentControlSocketPort = null;
+	private int myCurrentOffset = 0;
+
 
 	/*
 	 * Non instantiable
 	 */
-	private PortUtil() {
+	public PortUtil() {
 		// nothing
 	}
 
-	/**
+	public synchronized void clear() {
-	 * The entire purpose here is to find an available port that can then be
+		for (ServerSocket next : myControlSockets) {
-	 * bound for by server in a unit test without conflicting with other tests.
+			ourLog.info("Releasing control port: {}", next.getLocalPort());
-	 * <p>
-	 * This is really only used for unit tests but is included in the library
-	 * so it can be reused across modules. Use with caution.
-	 */
-	public static int findFreePort() {
-		ServerSocket server;
 			try {
-			server = new ServerSocket(0);
+				next.close();
+			} catch (IOException theE) {
+				// ignore
+			}
+		}
+		myControlSockets.clear();
+		myCurrentControlSocketPort = null;
+	}
+
+	public synchronized int getNextFreePort() {
+
+		while (true) {
+
+			// Acquire a control port
+			while (myCurrentControlSocketPort == null) {
+				int nextCandidate = (int) (Math.random() * 65000.0);
+				nextCandidate = nextCandidate - (nextCandidate % SPACE_SIZE);
+
+				if (nextCandidate < 10000) {
+					continue;
+				}
+
+				try {
+					ServerSocket server = new ServerSocket();
 					server.setReuseAddress(true);
-			int port = server.getLocalPort();
+					server.bind(new InetSocketAddress("localhost", nextCandidate));
+					myControlSockets.add(server);
+					ourLog.info("Acquired control socket on port {}", nextCandidate);
+					myCurrentControlSocketPort = nextCandidate;
+					myCurrentOffset = 0;
+				} catch (IOException theE) {
+					ourLog.info("Candidate control socket {} is already taken", nextCandidate);
+					continue;
+				}
+			}
 
-			/*
+			// Find a free port within the allowable range
-			 * Try to connect to the newly allocated port to make sure
+			while (true) {
-			 * it's free
+				myCurrentOffset++;
-			 */
+
-			for (int i = 0; i < 10; i++) {
+				if (myCurrentOffset == SPACE_SIZE) {
-				try {
+					// Current space is exhausted
-					Socket client = new Socket();
+					myCurrentControlSocketPort = null;
-					client.connect(new InetSocketAddress(port), 1000);
 					break;
-				} catch (Exception e) {
-					if (i == 9) {
-						throw new InternalErrorException("Can not connect to port: " + port);
-					}
-					Thread.sleep(250);
-				}
 				}
 
-			server.close();
+				int nextCandidatePort = myCurrentControlSocketPort + myCurrentOffset;
+
+				// Try to open a port on this socket and use it
+				try (ServerSocket server = new ServerSocket()) {
+					server.setReuseAddress(true);
+					server.bind(new InetSocketAddress("localhost", nextCandidatePort));
+					try (Socket client = new Socket()) {
+						client.setReuseAddress(true);
+						client.connect(new InetSocketAddress("localhost", nextCandidatePort));
+					}
+				} catch (IOException e) {
+					continue;
+				}
 
 				/*
 				 * This is an attempt to make sure the port is actually
@@ -89,7 +130,8 @@ public class PortUtil {
 				for (int i = 0; i < 10; i++) {
 					try {
 						Socket client = new Socket();
-					client.connect(new InetSocketAddress(port), 1000);
+						client.setReuseAddress(true);
+						client.connect(new InetSocketAddress(nextCandidatePort), 1000);
 						ourLog.info("Socket still seems open");
 						Thread.sleep(250);
 					} catch (Exception e) {
@@ -97,18 +139,34 @@ public class PortUtil {
 					}
 				}
 
-			// ....annnd sleep a bit for the same reason.
+//				try {
-			Thread.sleep(500);
+//					Thread.sleep(500);
+//				} catch (InterruptedException theE) {
+//					// ignore
+//				}
 
 				// Log who asked for the port, just in case that's useful
 				StackTraceElement[] stackTraceElements = Thread.currentThread().getStackTrace();
 				StackTraceElement previousElement = stackTraceElements[2];
-			ourLog.info("Returned available port {} for: {}", port, previousElement.toString());
+//				ourLog.info("Returned available port {} for: {}", nextCandidatePort, previousElement.toString());
+
+				return nextCandidatePort;
 
-			return port;
-		} catch (IOException | InterruptedException e) {
-			throw new Error(e);
 			}
+
+
+		}
+	}
+
+	/**
+	 * The entire purpose here is to find an available port that can then be
+	 * bound for by server in a unit test without conflicting with other tests.
+	 * <p>
+	 * This is really only used for unit tests but is included in the library
+	 * so it can be reused across modules. Use with caution.
+	 */
+	public static int findFreePort() {
+		return INSTANCE.getNextFreePort();
 	}
 
 }

hapi-fhir-base/src/test/java/ca/uhn/fhir/util/PortUtilTest.java

@@ -1,15 +1,26 @@
 package ca.uhn.fhir.util;
 
 import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.ServerSocket;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
 public class PortUtilTest {
 
+	private static final Logger ourLog = LoggerFactory.getLogger(PortUtilTest.class);
+
 	@Test
 	public void testFindFreePort() throws IOException {
 		int port = PortUtil.findFreePort();
@@ -28,4 +39,49 @@ public class PortUtilTest {
 		}
 	}
 
+	@Test
+	public void testPortsAreNotReused() throws InterruptedException {
+
+		List<Integer> ports = Collections.synchronizedList(new ArrayList<>());
+		List<PortUtil> portUtils = Collections.synchronizedList(new ArrayList<>());
+
+		int tasksCount = 50;
+		ExecutorService pool = Executors.newFixedThreadPool(tasksCount);
+		int portsPerTaskCount = 500;
+		for (int i = 0; i < tasksCount; i++) {
+			pool.submit(() -> {
+				PortUtil portUtil = new PortUtil();
+				portUtils.add(portUtil);
+				for (int j = 0; j < portsPerTaskCount; j++) {
+					int nextFreePort = portUtil.getNextFreePort();
+
+					try {
+						ServerSocket ss = new ServerSocket();
+						ss.bind(new InetSocketAddress("localhost", nextFreePort));
+					} catch (IOException e) {
+						ourLog.error("Failure binding new port: " + e.toString(), e);
+						fail(e.toString());
+					}
+
+					ports.add(nextFreePort);
+				}
+
+			});
+		}
+		pool.shutdown();
+		pool.awaitTermination(60, TimeUnit.SECONDS);
+
+		assertEquals(tasksCount * portsPerTaskCount, ports.size());
+
+		while (ports.size() > 0) {
+			Integer nextPort = ports.remove(0);
+			if (ports.contains(nextPort)) {
+				fail("Port " + nextPort + " was given out more than once");
+			}
+		}
+
+		for (PortUtil next : portUtils) {
+			next.clear();
+		}
+	}
 }