Target Explorer: Improve flexibility of the value-add launcher to customizations

2 files changed, 130 insertions, 37 deletions

diff --git a/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/AbstractExternalValueAdd.java b/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/AbstractExternalValueAdd.java
index 4c4bf5e1d..3f7b36513 100644
--- a/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/AbstractExternalValueAdd.java
+++ b/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/AbstractExternalValueAdd.java
@@ -9,8 +9,10 @@
  *******************************************************************************/
 package org.eclipse.tcf.te.tcf.core.va;
 
+import java.io.BufferedWriter;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.OutputStreamWriter;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -55,9 +57,33 @@ public abstract class AbstractExternalValueAdd extends AbstractValueAdd {
 		 */
 		@Override
 		public void dispose() {
-			if (process != null) process.destroy();
-			if (outputReader != null) outputReader.interrupt();
-			if (errorReader != null) errorReader.interrupt();
+			if (process != null) {
+				// Send "quit" to the process before we destroy the process
+				BufferedWriter writer = null;
+				try {
+					writer = new BufferedWriter(new OutputStreamWriter(process.getOutputStream()));
+					writer.write("quit"); //$NON-NLS-1$
+
+					// Check the process exit code
+					long start = System.currentTimeMillis();
+					while ((start + 2000) >= System.currentTimeMillis()) {
+						try {
+							process.exitValue();
+							process = null;
+							break;
+						} catch (IllegalThreadStateException e) {
+							try { Thread.sleep(200); } catch (InterruptedException ex) { /* ignored on purpose */ }
+						}
+					}
+				} catch (IOException e) {
+					/* ignored on purpose */
+				} finally {
+					if (writer != null) { try { writer.close(); } catch (IOException e) { /* ignored on purpose */} writer = null; }
+					if (process != null) { process.destroy(); process = null; }
+				}
+			}
+			if (outputReader != null) { outputReader.interrupt(); outputReader = null; }
+			if (errorReader != null) { errorReader.interrupt(); errorReader = null; }
 		}
 	}
 
diff --git a/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/ValueAddLauncher.java b/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/ValueAddLauncher.java
index b7a4949ee..be300279f 100644
--- a/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/ValueAddLauncher.java
+++ b/target_explorer/plugins/org.eclipse.tcf.te.tcf.core/src/org/eclipse/tcf/te/tcf/core/va/ValueAddLauncher.java
@@ -9,6 +9,7 @@
  *******************************************************************************/
 package org.eclipse.tcf.te.tcf.core.va;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.ServerSocket;
@@ -65,11 +66,20 @@ public class ValueAddLauncher extends ProcessLauncher {
 	}
 
 	/**
+	 * Returns the target peer id.
+	 *
+	 * @return The target peer id.
+	 */
+	protected final String getPeerId() {
+		return id;
+	}
+
+	/**
 	 * Returns the process handle.
 	 *
 	 * @return The process handle or <code>null</code>.
 	 */
-	public Process getProcess() {
+	public final Process getProcess() {
 		return process;
 	}
 
@@ -78,7 +88,7 @@ public class ValueAddLauncher extends ProcessLauncher {
 	 *
 	 * @return The process output reader or <code>null</code>.
 	 */
-	public ProcessOutputReaderThread getOutputReader() {
+	public final ProcessOutputReaderThread getOutputReader() {
 		return outputReader;
 	}
 
@@ -87,7 +97,7 @@ public class ValueAddLauncher extends ProcessLauncher {
 	 *
 	 * @return The process error reader or <code>null</code>.
 	 */
-	public ProcessOutputReaderThread getErrorReader() {
+	public final ProcessOutputReaderThread getErrorReader() {
 		return errorReader;
 	}
 
@@ -99,38 +109,14 @@ public class ValueAddLauncher extends ProcessLauncher {
 		IPath dir = path.removeLastSegments(1);
 		String cmd = Host.isWindowsHost() ? path.toOSString() : "./" + path.lastSegment(); //$NON-NLS-1$
 
-		// Determine a free port to use by the value-add. We must
-		// avoid to launch the value-add at the default port 1534.
-		int port = getFreePort();
-
 		// Build up the command
 		List<String> command = new ArrayList<String>();
 		command.add(cmd);
-		addToCommand(command, "-I180"); //$NON-NLS-1$
-		addToCommand(command, "-S"); //$NON-NLS-1$
-		addToCommand(command, "-sTCP::" + (port != -1 ? Integer.valueOf(port) : "") + ";ValueAdd=1"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
 
-		// Enable logging?
-		if (Boolean.getBoolean("va.logging.enable")) { //$NON-NLS-1$
-			// Calculate the location and name of the log file
-			Bundle bundle = Platform.getBundle("org.eclipse.tcf.te.tcf.log.core"); //$NON-NLS-1$
-			IPath location = bundle != null ? Platform.getStateLocation(bundle) : null;
-			if (location != null) {
-				location = location.append(".logs"); //$NON-NLS-1$
-
-				String name = "Output_" + valueAddId + "_" + id + ".log"; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
-				name = name.replaceAll("\\s", "_"); //$NON-NLS-1$ //$NON-NLS-2$
-				name = name.replaceAll("[:/\\;,]", "_"); //$NON-NLS-1$ //$NON-NLS-2$
-
-				location = location.append(name);
-				addToCommand(command, "-L" + location.toString()); //$NON-NLS-1$
-
-				String level = System.getProperty("va.logging.level"); //$NON-NLS-1$
-				if (level != null && !"".equals(level.trim())) { //$NON-NLS-1$
-					addToCommand(command, "-l" + level.trim()); //$NON-NLS-1$
-				}
-			}
-		}
+		// Add command line parameters for the value-add
+		addCommandLineParameters(command);
+		// Add the logging command line parameters for the value-add
+		addLoggingCommandLineParameters(command);
 
 		if (CoreBundleActivator.getTraceHandler().isSlotEnabled(0, ITraceIds.TRACE_CHANNEL_MANAGER)) {
 			CoreBundleActivator.getTraceHandler().trace(NLS.bind(Messages.ValueAddLauncher_launch_command, new Object[] { command, id, valueAddId }),
@@ -148,14 +134,14 @@ public class ValueAddLauncher extends ProcessLauncher {
 		}
 
 		// Launch the value-add
-		process = Runtime.getRuntime().exec(command.toArray(new String[command.size()]), envp, dir.toFile());
+		process = exec(command.toArray(new String[command.size()]), envp, dir.toFile());
 
 		// Launch the process output reader
-		outputReader = new ProcessOutputReaderThread(path.lastSegment(), new InputStream[] { process.getInputStream() });
+		outputReader = createProcessOutputReaderThread(path, process.getInputStream());
 		outputReader.start();
 
 		// Launch the process error reader
-		errorReader = new ProcessOutputReaderThread(path.lastSegment(), new InputStream[] { process.getErrorStream() });
+		errorReader = createProcessOutputReaderThread(path, process.getErrorStream());
 		errorReader.start();
 	}
 
@@ -175,6 +161,87 @@ public class ValueAddLauncher extends ProcessLauncher {
 	}
 
 	/**
+	 * Add the value-add command line parameters to the command.
+	 *
+	 * @param command The command. Must not be <code>null</code>.
+	 * @throws Throwable In case something failed while adding the command line parameters.
+	 */
+	protected void addCommandLineParameters(List<String> command) throws Throwable {
+		Assert.isNotNull(command);
+
+		// Determine a free port to use by the value-add. We must
+		// avoid to launch the value-add at the default port 1534.
+		int port = getFreePort();
+
+		addToCommand(command, "-I180"); //$NON-NLS-1$
+		addToCommand(command, "-S"); //$NON-NLS-1$
+		addToCommand(command, "-sTCP::" + (port != -1 ? Integer.valueOf(port) : "") + ";ValueAdd=1"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+	}
+
+	/**
+	 * Add the value-add logging command line parameters to the command.
+	 *
+	 * @param command The command. Must not be <code>null</code>.
+	 * @throws Throwable In case something failed while adding the logging command line parameters.
+	 */
+	protected void addLoggingCommandLineParameters(List<String> command) throws Throwable {
+		Assert.isNotNull(command);
+
+		// Enable logging?
+		if (Boolean.getBoolean("va.logging.enable")) { //$NON-NLS-1$
+			// Calculate the location and name of the log file
+			Bundle bundle = Platform.getBundle("org.eclipse.tcf.te.tcf.log.core"); //$NON-NLS-1$
+			IPath location = bundle != null ? Platform.getStateLocation(bundle) : null;
+			if (location != null) {
+				location = location.append(".logs"); //$NON-NLS-1$
+
+				String name = "Output_" + valueAddId + "_" + id + ".log"; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+				name = name.replaceAll("\\s", "_"); //$NON-NLS-1$ //$NON-NLS-2$
+				name = name.replaceAll("[:/\\;,]", "_"); //$NON-NLS-1$ //$NON-NLS-2$
+
+				location = location.append(name);
+				addToCommand(command, "-L" + location.toString()); //$NON-NLS-1$
+
+				String level = System.getProperty("va.logging.level"); //$NON-NLS-1$
+				if (level != null && !"".equals(level.trim())) { //$NON-NLS-1$
+					addToCommand(command, "-l" + level.trim()); //$NON-NLS-1$
+				}
+			}
+		}
+	}
+
+	/**
+	 * Execute the value-add launch command.
+	 *
+	 * @param cmdarray Array containing the command to call and its arguments. Must not be <code>null</code>.
+	 * @param envp Array of strings, each element of which has environment variable settings in the format <code>name=value</code>,
+	 *             or <code>null</code> if the subprocess should inherit the environment of the current process.
+	 * @param dir The working directory of the subprocess, or <code>null</code> if the subprocess should inherit
+	 *            the working directory of the current process.
+	 *
+	 * @return The process instance.
+	 * @see Runtime#exec(String[], String[], File)
+	 */
+	protected Process exec(String[] cmdarray, String[] envp, File dir) throws IOException {
+		Assert.isNotNull(cmdarray);
+		return Runtime.getRuntime().exec(cmdarray, envp, dir);
+	}
+
+	/**
+	 * Creates the process output reader thread for the given stream.
+	 *
+	 * @param path The process path. Must not be <code>null</code>.
+	 * @param stream The stream. Must not be <code>null</code>.
+	 *
+	 * @return The not yet started process output reader thread instance.
+	 */
+	protected ProcessOutputReaderThread createProcessOutputReaderThread(IPath path, InputStream stream) {
+		Assert.isNotNull(path);
+		Assert.isNotNull(stream);
+		return new ProcessOutputReaderThread(path.lastSegment(), new InputStream[] { stream });
+	}
+
+	/**
 	 * Determine a free port to use.
 	 *
 	 * @return A free port or <code>-1</code>.