Auto-generate fork binding ports in the forked process by default

This fixes issues #177 and #176. It turns around the fork process port
auto-determination: instead of trying out ports on the master process,
then communicating a port to a fork and hope that it is still free when
the fork has finished starting up, the forked process now finds a free
port to bind to and communicates this to the master via a special log
message (STDOUT) that is understood and parsed by the master process.
Implementors of different forking schemes (like distributed ones with
forks on different machines) might have to make minor adjustments,
especially if they chose to not forward stdout streams to the master (in
that case they will now have to take care of communicating the forks'
port back to the master).

de.gebit.integrity.remoting/src/de/gebit/integrity/remoting/server/IntegrityRemotingServer.java

@@ -79,7 +79,7 @@ public class IntegrityRemotingServer {
 	 * @param aHostIP
 	 *            the host IP to listen on
 	 * @param aPort
-	 *            the port to listen on
+	 *            the port to listen on (0 = auto-choose a free port)
 	 * @param aListener
 	 *            the listener
 	 * @param aClassLoader
@@ -95,9 +95,9 @@ public IntegrityRemotingServer(String aHostIP, int aPort, IntegrityRemotingServe
 			throw new IllegalArgumentException("A listener must be provided.");
 		}
 		listener = aListener;
-		port = aPort;
 		isFork = anIsForkFlag;
 		serverEndpoint = new ServerEndpoint(aHostIP, aPort, createProcessors(), aClassLoader, anIsForkFlag);
+		port = serverEndpoint.getPort();
 	}
 
 	/**

de.gebit.integrity.remoting/src/de/gebit/integrity/remoting/transport/ServerEndpoint.java

@@ -65,7 +65,7 @@ public class ServerEndpoint {
 	 * @param aHostIP
 	 *            the host IP to listen on
 	 * @param aPort
-	 *            the port to bind to
+	 *            the port to bind to (0 = auto-choose a free port)
 	 * @param aProcessorMap
 	 *            the map of processors to use for processing incoming messages
 	 * @param aClassLoader
@@ -77,8 +77,7 @@ public class ServerEndpoint {
 	 */
 	public ServerEndpoint(String aHostIP, int aPort,
 			Map<Class<? extends AbstractMessage>, MessageProcessor<?>> aProcessorMap, ClassLoader aClassLoader,
-			boolean anIsForkFlag)
-			throws UnknownHostException, IOException {
+			boolean anIsForkFlag) throws UnknownHostException, IOException {
 		messageProcessors = aProcessorMap;
 		serverSocket = new ServerSocket(aPort, 0, Inet4Address.getByName(aHostIP));
 		connectionWaiter = new ConnectionWaiter(aClassLoader);
@@ -95,6 +94,15 @@ public boolean isActive() {
 		return serverSocket.isBound() && !serverSocket.isClosed();
 	}
 
+	/**
+	 * Returns the port number on which the server endpoint is listening.
+	 * 
+	 * @return
+	 */
+	public int getPort() {
+		return serverSocket.getLocalPort();
+	}
+
 	/**
 	 * Closes the server endpoint and all endpoints currently active.
 	 * 
@@ -122,7 +130,7 @@ public void closeAll(boolean anEmptyOutputQueueFlag) {
 
 		if (shutdownHookThread != null) {
 			try {
-			Runtime.getRuntime().removeShutdownHook(shutdownHookThread);
+				Runtime.getRuntime().removeShutdownHook(shutdownHookThread);
 			} catch (IllegalStateException exc) {
 				// ignored - may be thrown if this code is run during VM shutdown, but we don't care about that
 			}

de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java

@@ -591,26 +591,58 @@ public void initialize(TestModel aModel, Map<String, String> someParameterizedCo
 
 		parameterizedConstantValues = someParameterizedConstants;
 		commandLineArguments = someCommandLineArguments;
+
+		performRemotingBinding(aRemotingPort, aRemotingBindHost);
+
+		waitForSetListInjection();
+	}
+
+	/**
+	 * Performs the remoting port binding.
+	 * 
+	 * @param aRemotingPort
+	 *            the port to bind to, or null if no remoting is desired
+	 * @param aRemotingBindHost
+	 *            the host/IP to bind to
+	 * @throws IOException
+	 */
+	protected void performRemotingBinding(Integer aRemotingPort, String aRemotingBindHost) throws IOException {
 		Integer tempRemotingPort = aRemotingPort;
-		String tempRemotingBindHost = aRemotingBindHost;
-		if (isFork()) {
+		String tempRemotingHost = aRemotingBindHost;
+		if (isFork() && System.getProperty(Forker.SYSPARAM_FORK_REMOTING_HOST) != null) {
+			tempRemotingHost = System.getProperty(Forker.SYSPARAM_FORK_REMOTING_HOST);
+		}
+		if (isFork() && System.getProperty(Forker.SYSPARAM_FORK_REMOTING_PORT) != null) {
 			tempRemotingPort = Integer.parseInt(System.getProperty(Forker.SYSPARAM_FORK_REMOTING_PORT));
-			tempRemotingBindHost = System.getProperty(Forker.SYSPARAM_FORK_REMOTING_HOST, aRemotingBindHost);
 		}
+
 		if (tempRemotingPort != null) {
 			remotingListener = new RemotingListener();
 			try {
-				remotingServer = new IntegrityRemotingServer(tempRemotingBindHost, tempRemotingPort, remotingListener,
+				remotingServer = new IntegrityRemotingServer(tempRemotingHost, tempRemotingPort, remotingListener,
 						javaClassLoader, isFork());
 			} catch (BindException exc) {
-				System.err.println("FAILED TO BIND REMOTING SERVER TO " + aRemotingBindHost + ":" + aRemotingPort);
+				System.err.println("FAILED TO BIND REMOTING SERVER TO " + tempRemotingHost + ":" + tempRemotingPort);
 				throw exc;
 			}
+
+			if (tempRemotingPort == 0) {
+				// This message is important! It is being parsed by the master to find out the auto-generated fork port
+				// (see de.gebit.integrity.runner.forking.Fork.FORK_HOST_AND_PORT_PATTERN).
+				System.out
+						.println("Integrity Test Runner bound to " + tempRemotingHost + ":" + remotingServer.getPort());
+			}
 		}
+	}
 
+	/**
+	 * If this is a fork, we now need to wait for the setlist and test scripts to be injected by the master! Otherwise
+	 * this method returns immediately.
+	 * 
+	 * @throws IOException
+	 */
+	protected void waitForSetListInjection() throws IOException {
 		if (isFork()) {
-			// If this is a fork, we now need to wait for the setlist and test scripts to be injected by
-			// the master!
 			long tempTimeout = System.nanoTime() + TimeUnit.SECONDS.toNanos(getForkConnectionTimeout());
 
 			synchronized (setListWaiter) {

de.gebit.integrity.runner/src/de/gebit/integrity/runner/console/ConsoleTestExecutor.java

@@ -200,7 +200,7 @@ public int run(String[] someArgs) {
 				"variant", "Specify the variant to execute (must be defined in the scripts!)", "[{-v,--variant}]");
 		SimpleCommandLineParser.BooleanOption tempNoremoteOption = new SimpleCommandLineParser.BooleanOption(null,
 				"noremote", "Disables remoting", "[{--noremote}]");
-		SimpleCommandLineParser.IntegerOption tempRemoteportOption = new SimpleCommandLineParser.IntegerOption("r",
+		SimpleCommandLineParser.IntegerOption tempRemotePortOption = new SimpleCommandLineParser.IntegerOption("r",
 				"remoteport", "Set the port number to bind to for remoting (default is "
 						+ IntegrityRemotingConstants.DEFAULT_PORT + ")",
 				"[{-r,--remoteport} port]");
@@ -222,7 +222,7 @@ public int run(String[] someArgs) {
 				null, "noconsole", "Do not capture stdout & stderr for test XML/HTML output", "[{--noconsole}]");
 
 		tempParser.addOptions(tempConsoleOption, tempXmlOption, tempXsltOption, tempNameOption, tempVariantOption,
-				tempNoremoteOption, tempRemoteportOption, tempRemoteHostOption, tempWaitForPlayOption,
+				tempNoremoteOption, tempRemotePortOption, tempRemoteHostOption, tempWaitForPlayOption,
 				tempSkipModelCheck, tempParameterizedConstantOption, tempSeedOption, tempExcludeConsoleStreamsOption);
 
 		if (someArgs.length == 0) {
@@ -301,7 +301,7 @@ public int run(String[] someArgs) {
 			Integer tempRemotePort = null;
 			String tempRemoteHost = null;
 			if (!tempNoremoteOption.isSet()) {
-				tempRemotePort = tempRemoteportOption.getValue(IntegrityRemotingConstants.DEFAULT_PORT);
+				tempRemotePort = tempRemotePortOption.getValue(IntegrityRemotingConstants.DEFAULT_PORT);
 				tempRemoteHost = tempRemoteHostOption.getValue("0.0.0.0");
 			}
 

de.gebit.integrity.runner/src/de/gebit/integrity/runner/forking/DefaultForker.java

@@ -11,11 +11,6 @@
 import java.io.IOException;
 import java.lang.management.ManagementFactory;
 import java.lang.management.RuntimeMXBean;
-import java.net.DatagramSocket;
-import java.net.Inet4Address;
-import java.net.InetAddress;
-import java.net.ServerSocket;
-import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map.Entry;
@@ -38,34 +33,29 @@ public class DefaultForker implements Forker {
 	@Override
 	public ForkedProcess fork(String[] someCommandLineArguments, String aForkName, long aRandomSeed)
 			throws ForkException {
-		int tempPortNumber = getPortToUse();
-
-		List<String> tempArgs = createArgumentList(someCommandLineArguments, tempPortNumber, aForkName, aRandomSeed);
-		return createProcess(tempArgs, tempPortNumber);
+		List<String> tempArgs = createArgumentList(someCommandLineArguments, aForkName, aRandomSeed);
+		return createProcess(tempArgs);
 	}
 
 	/**
 	 * Creates the argument list used to fork the process.
 	 * 
 	 * @param someCommandLineArguments
 	 *            the command line arguments provided to {@link #fork(String[], int, String)}.
-	 * @param aPortNumber
-	 *            the port number to use by the fork
 	 * @param aForkName
 	 *            the name for the new fork
 	 * @param aRandomSeed
 	 *            the seed for the RNG of the fork
 	 * @return the argument list
 	 */
-	protected List<String> createArgumentList(String[] someCommandLineArguments, int aPortNumber, String aForkName,
-			long aRandomSeed) {
+	protected List<String> createArgumentList(String[] someCommandLineArguments, String aForkName, long aRandomSeed) {
 		List<String> tempArgs = new ArrayList<String>();
 
 		addJavaExecutable(tempArgs);
 
 		addClassPath(tempArgs);
 
-		addForkInformation(tempArgs, aPortNumber, aForkName, aRandomSeed);
+		addForkInformation(tempArgs, aForkName, aRandomSeed);
 
 		addJVMArguments(tempArgs);
 
@@ -107,24 +97,41 @@ protected void addClassPath(List<String> anArgumentList) {
 	 * 
 	 * @param anArgumentList
 	 *            the argument list to extend
-	 * @param aPortNumber
-	 *            the port number for the fork
 	 * @param aForkName
 	 *            the name for the fork
 	 * @param aRandomSeed
 	 *            the seed for the RNG of the fork
 	 */
-	protected void addForkInformation(List<String> anArgumentList, int aPortNumber, String aForkName,
-			long aRandomSeed) {
-		String tempHostInterface = getHostInterfaceToUse();
-		if (tempHostInterface != null) {
-			anArgumentList.add("-D" + Forker.SYSPARAM_FORK_REMOTING_HOST + "=" + tempHostInterface);
+	protected void addForkInformation(List<String> anArgumentList, String aForkName, long aRandomSeed) {
+		if (getForkHost() != null) {
+			anArgumentList.add("-D" + Forker.SYSPARAM_FORK_REMOTING_HOST + "=" + getForkHost());
+		}
+		if (getForkPort() != null) {
+			anArgumentList.add("-D" + Forker.SYSPARAM_FORK_REMOTING_PORT + "=" + getForkPort());
 		}
-		anArgumentList.add("-D" + Forker.SYSPARAM_FORK_REMOTING_PORT + "=" + aPortNumber);
+
 		anArgumentList.add("-D" + Forker.SYSPARAM_FORK_NAME + "=" + aForkName);
 		anArgumentList.add("-D" + Forker.SYSPARAM_FORK_SEED + "=" + aRandomSeed);
 	}
 
+	/**
+	 * The host name / IP to which the fork should be bound.
+	 * 
+	 * @return
+	 */
+	protected String getForkHost() {
+		return "localhost";
+	}
+
+	/**
+	 * The port to which the fork should be bound.
+	 * 
+	 * @return
+	 */
+	protected Integer getForkPort() {
+		return 0;
+	}
+
 	/**
 	 * Adds any random JVM configuration arguments to the argument list. Default implementation asks the
 	 * {@link RuntimeMXBean}.
@@ -177,10 +184,10 @@ protected void addApplicationArguments(List<String> anArgumentList, String[] som
 	 * @return the forked process
 	 * @throws ForkException
 	 */
-	protected LocalForkedProcess createProcess(List<String> anArgumentList, int aPortNumber) throws ForkException {
+	protected LocalForkedProcess createProcess(List<String> anArgumentList) throws ForkException {
 		ProcessBuilder tempBuilder = new ProcessBuilder(anArgumentList);
 		try {
-			return new LocalForkedProcess(tempBuilder.start(), aPortNumber);
+			return new LocalForkedProcess(tempBuilder.start());
 		} catch (IOException exc) {
 			throw new ForkException("Error forking process", exc);
 		}
@@ -202,79 +209,4 @@ protected String guessMainClassName() {
 				"Could not determine main class name. You probably need to implement your own Forker in order to use forking!");
 	}
 
-	/**
-	 * The maximum possible port number.
-	 */
-	private static final int MAX_PORT_NUMBER = 65535;
-
-	/**
-	 * The minimum possible port number.
-	 */
-	private static final int MIN_PORT_NUMBER = 1024;
-
-	/**
-	 * This determines the port to use by the fork to communicate with the master. The fork must be able to bind to this
-	 * port. The default implementation finds a free port on the machine by randomly checking ports above
-	 * {@link #MIN_PORT_NUMBER}.
-	 * 
-	 * @return the port to use
-	 */
-	protected int getPortToUse() {
-		int tempPort = 0;
-		do {
-			tempPort = (int) Math.floor(Math.random() * (double) (MAX_PORT_NUMBER - MIN_PORT_NUMBER)) + MIN_PORT_NUMBER;
-		} while (!isPortAvailable(tempPort));
-		return tempPort;
-	}
-
-	/**
-	 * Determines the host interface to bind the fork to.
-	 * 
-	 * @return the host interface name (IP or hostname)
-	 */
-	protected String getHostInterfaceToUse() {
-		return "localhost";
-	}
-
-	/**
-	 * Checks whether a given port is available on the local machine.
-	 * 
-	 * @param aPort
-	 *            the port to check
-	 * @return true if the port is available, false if not
-	 */
-	protected boolean isPortAvailable(int aPort) {
-		InetAddress tempInterface;
-		try {
-			tempInterface = Inet4Address.getByName("localhost");
-		} catch (UnknownHostException exc1) {
-			// This is almost impossible to occur!
-			throw new RuntimeException(exc1);
-		}
-		ServerSocket tempServerSocket = null;
-		DatagramSocket tempDatagramSocket = null;
-		try {
-			tempServerSocket = new ServerSocket(aPort, 1, tempInterface);
-			tempServerSocket.setReuseAddress(true);
-			tempDatagramSocket = new DatagramSocket(aPort, tempInterface);
-			tempDatagramSocket.setReuseAddress(true);
-			return true;
-		} catch (IOException exc) {
-			// nothing to do
-		} finally {
-			if (tempDatagramSocket != null) {
-				tempDatagramSocket.close();
-			}
-
-			if (tempServerSocket != null) {
-				try {
-					tempServerSocket.close();
-				} catch (IOException exc) {
-					// ignore
-				}
-			}
-		}
-
-		return false;
-	}
 }