Commits

Brendan Humphreys committed 9f84bb2

NONE: introduction of interface and factory to allow external processes to be more easily mocked

Comments (0)

Files changed (7)

processutils/src/main/java/com/atlassian/utils/process/ExternalProcess.java

 package com.atlassian.utils.process;
 
-import java.io.File;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.SynchronousQueue;
-import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
+public interface ExternalProcess extends Watchdog {
 
-import org.apache.log4j.Logger;
+    public ProcessHandler getHandler();
 
-/**
- * This class manages the execution of an external process, using separate threads to process
- * the process' IO requirements.
- */
-public class ExternalProcess implements Watchdog {
-    private String[] cmdArray;
-    private File workingDir;
-    private String[] environment;
-    private ProcessHandler handler;
-    private Process process;
+    public Long getStartTime();
 
-    private List<ProcessMonitor> monitors = new ArrayList<ProcessMonitor>();
-    
-    private ProcessException processException;
+    public void start();
 
-    private LatchedRunnable outputPump;
-    private LatchedRunnable errorPump;
-    private LatchedRunnable inputPump;
+    public void finish();
 
-    private static final ExecutorService pumpThreadPool;
+    public boolean finish(int maxWait);
 
-    private long lastWatchdogReset;
-    private long timeout = 60000L;
-    private Long startTime;
-    private boolean cancelled;
-    
-    public void resetWatchdog() {
-        lastWatchdogReset = System.currentTimeMillis();
-    }
+    public void execute();
 
-    public long getTimeoutTime() {
-        return lastWatchdogReset + timeout;
-    }
+    public void executeWhile(Runnable runnable);
 
-    public boolean isTimedOut() {
-        return getTimeoutTime() < System.currentTimeMillis();
-    }
-
-    static {
-        ThreadFactory threadFactory = new ThreadFactory() {
-            public Thread newThread(Runnable r) {
-                return new Thread(r, "ExtProcess IO Pump");
-            }
-        };
-        pumpThreadPool = new ThreadPoolExecutor(6, Integer.MAX_VALUE, 120, TimeUnit.SECONDS,
-                                                new SynchronousQueue<Runnable>(), threadFactory);
-    }
-
-    /**
-     * Process an external command.
-     * @param cmdArray the command and its arguments as separate elements
-     * @param handler The handler for this execution. The handler supports the required IO
-     *                operations
-     */
-    public ExternalProcess(String[] cmdArray, ProcessHandler handler) {
-        setCommand(cmdArray);
-        setHandler(handler);
-    }
-
-    /**
-     * Process an external command (the command and arguments are given as a list)
-     * @param command A list containing the command and its arguments
-     * @param handler The process handler to manage the execution of this process.
-     */
-    public ExternalProcess(List<String> command, ProcessHandler handler) {
-        setCommand(command.toArray(new String[command.size()]));
-        setHandler(handler);
-    }
-
-    /**
-     * Process an external command. The command is given as a single command line and parsed into
-     * the command and its arguments. Spaces are used as argument delimiters so if any command arguments
-     * need to contain spaces, the array or list based constructors should be used.
-     *
-     * @param commandLine the command and its arguments in a single line. If any arguments
-     *                    need to contain spaces, the array or list based constructors should be used.
-     * @param handler The handler for this execution. The handler supports the required IO
-     *                operations
-     */
-    public ExternalProcess(String commandLine, ProcessHandler handler) {
-        String[] cmdArray = ProcessUtils.tokenizeCommand(commandLine);
-        setCommand(cmdArray);
-        setHandler(handler);
-    }
-
-    private void setHandler(ProcessHandler handler) {
-        this.handler = handler;
-    }
-
-    private void setCommand(String[] cmdArray) {
-        this.cmdArray = cmdArray;
-    }
-
-    public void setWorkingDir(File workingDir) {
-        this.workingDir = workingDir;
-    }
-
-    public void setEnvironment(String[] environment) {
-        this.environment = environment;
-    }
-
-    private boolean arePumpsRunning() {
-        return outputPump.isRunning() || errorPump.isRunning()
-                || (inputPump != null && inputPump.isRunning());
-    }
-
-    /**
-     * Get the process handler for this process execution
-     *
-     * @return the ProcessHandler instance associated with this process execution.
-     */
-    public ProcessHandler getHandler() {
-        return handler;
-    }
-
-    /**
-     * @return the time process execution started. null if the process has not yet started.
-     */
-    public Long getStartTime() {
-        return this.startTime;
-    }
-    
-    public void addMonitor(ProcessMonitor monitor) {
-        this.monitors.add(monitor);
-    }
-    
-    public void removeMonitor(ProcessMonitor monitor) {
-        this.monitors.remove(monitor);
-    }
-    
-    private boolean isWindows() {
-    	return System.getProperty("os.name").toLowerCase().contains("windows");
-    }
-    
-    private String quoteString(String value) {
-        StringBuilder builder = new StringBuilder()
-            .append("\"")
-            .append(value.replace("\"", "\\\""))
-            .append("\"");
-        return builder.toString();
-    }
-    
-    /*
-      * This method provides a workaround for a JVM bug on windows (see http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4947220). The bug
-      * is that the Sun/Oracle JVM uses the (8 bit) OEM codepage for encoding commandline arguments that are passed to an external process.  Any
-      * characters in a command line argument that can't be represented in the OEM codepage will be replaced by the '?' character, which will probably
-      * cause the command that's being called to fail.
-      * 
-      * A bit of background information is helpful to understand what's going on. Windows uses 2 code pages: the OEM code page and the ANSI code page 
-      * (or 'Windows code page'). The OEM code page is always limited to 8 bit character encodings, whereas some windows code pages are 8 bit and some
-      * are larger. The OEM code page is typically used for console applications, whereas the windows code pages are used for native non-Unicode application
-      * using a GUI on Windows systems.  The system-wide settings can be found in the windows registry:
-      * 
-      * 
-      * More info about the history of OEM vs ANSI code pages: http://blogs.msdn.com/b/michkap/archive/2005/02/08/369197.aspx
-      * 
-      * The workaround is to store the command-line arguments in environment variables and refer to the environment vars on the command line. Windows
-      * cmd will expand the command line arguments without using to the OEM code page, which usually has more correlation with the locale of the 
-      * producer of the hardware (it's derived from the BIOS code page) than with the regional settings configured in Windows. The ANSI code page is derived from
-      * the regional settings.
-      * 
-      * When cmd expands the %JENV_XXX% vars on the command line it uses the ANSI code page instead of the OEM code page (or that is what testing
-      * seems to indicate, can't find any definitive answer in the cmd docos). While this still isn't a true fix to the problem, for most situations this will be sufficient 
-      * as the user typically won't use characters that aren't defined for his locale. But then again, they might..
-     */
-    private Process createWinProcess(String[] cmdArray, String[] environment, File workingDir) throws IOException {
-        Map<String, String> newEnv = new HashMap<String, String>();
-        if (environment == null) {
-            // inherit the environment of the current process
-            newEnv.putAll(System.getenv());
-        } else {
-            // add the env entries
-            for (String env: environment) {
-                String[] values = env.split("=");
-                if (values.length > 1) {
-                    newEnv.put(values[0], values[1]);
-                }
-            }
-        }
-        int extraArgs = 3;
-        String[] i18n = new String[cmdArray.length + extraArgs];
-        i18n[0] = "cmd";
-        i18n[1] = "/A"; // use ANSI encoding 
-        i18n[2] = "/C";
-        i18n[extraArgs] = cmdArray[0];
-        for (int counter = 1; counter < cmdArray.length; counter++) {
-            String envName = "JENV_" + counter;
-            i18n[counter + extraArgs] = "%" + envName + "%";
-            newEnv.put(envName, quoteString(cmdArray[counter]));
-        }
-        cmdArray = i18n;
-
-        ProcessBuilder pb = new ProcessBuilder(cmdArray);
-        pb.directory(workingDir);
-        Map<String, String> env = pb.environment();
-        env.putAll(newEnv);
-        return pb.start();
-    }
-    
-    private Process createProcess(String[] cmdArray, String[] environment, File workingDir) throws IOException {
-    	if (isWindows()) {
-    		return createWinProcess(cmdArray, environment, workingDir);
-    	} else {
-    		return Runtime.getRuntime().exec(cmdArray, environment, workingDir);
-    	}
-    }
-    
-    /**
-     * Start the external process and setup the IO pump threads needed to
-     * manage the process IO. If you call this method you must eventually call the
-     * finish() method. Using this method you may execute additional code between process
-     * start and finish.
-     */
-    public void start() {
-        try {
-            this.startTime = System.currentTimeMillis();
-            this.process = createProcess(cmdArray, environment, workingDir);
-            setupIOPumps();
-        } catch (IOException e) {
-            processException = new ProcessException(e);
-        }
-    }
-
-    private void setupIOPumps() {
-        // set up threads to feed data to and extract data from the process
-        if (handler.hasInput()) {
-            inputPump = new LatchedRunnable() {
-                protected void doTask() {
-                    handler.provideInput(process.getOutputStream());
-                }
-            };
-        }
-
-        errorPump = new LatchedRunnable() {
-            protected void doTask() {
-                try {
-                    handler.processError(process.getErrorStream());
-                } catch (Throwable e) {
-                    if (!isCancelled()) {
-                        processException = new ProcessException(e);
-                    }
-                }
-            }
-        };
-
-        outputPump = new LatchedRunnable() {
-            protected void doTask() {
-                try {
-                    handler.processOutput(process.getInputStream());
-                } catch (Throwable e) {
-                    if (!isCancelled()) {
-                        processException = new ProcessException(e);
-                    }
-                }
-            }
-        };
-
-        // tickle the dog initially
-        resetWatchdog();
-        handler.setWatchdog(this);
-
-        pumpThreadPool.execute(errorPump);
-        pumpThreadPool.execute(outputPump);
-        if (inputPump != null) {
-            pumpThreadPool.execute(inputPump);
-        }
-    }
-
-    /**
-     * Finish process execution. This method should be called after you have called the
-     * start() method.
-     */
-    public void finish() {
-        if (process != null) {
-            try {
-                do {
-                    long checkTime = getTimeoutTime();
-                    awaitPump(outputPump, checkTime);
-                    awaitPump(inputPump, checkTime);
-                    awaitPump(errorPump, checkTime);
-                } while (!isTimedOut() && arePumpsRunning());
-            } finally {
-                int exitCode = 0;
-                if (!cancelled) {
-                    exitCode = wrapUpProcess();
-                }
-                handler.complete(exitCode, processException);
-            }
-        } else {
-            handler.complete(-1, processException);
-        }
-    }
-    
-    /**
-     * Notifies all ProcessMonitors of the 'beforeStart' event.
-     */
-    private void notifyBeforeStart() {
-        for (ProcessMonitor monitor: monitors) {
-            try {
-                monitor.onBeforeStart(this);
-            } catch (Exception e) {
-                // catch and log error, but continue
-                Logger.getLogger(ExternalProcess.class).error("Error while processing 'beforeStarted' event:", e);
-            }
-        }
-    }
-
-    /**
-     * Notifies all ProcessMonitors of the 'afterFinished' event.
-     */
-    private void notifyAfterFinished() {
-        for (ProcessMonitor monitor: monitors) {
-            try {
-                monitor.onAfterFinished(this);
-            } catch (Exception e) {
-                Logger.getLogger(ExternalProcess.class).error("Error while processing 'afterFinished' event:", e);
-            }
-        }
-    }
-    
-    /**
-     * Execute the external command. When this method returns, the process handler
-     * provided at construction time should be consulted to collect exit code, exceptions,
-     * process output, etc.
-     */
-    public void execute() {
-        notifyBeforeStart();
-        try {
-            start();
-            finish();
-        } finally {
-            notifyAfterFinished();
-        }
-    }
-
-    /**
-     * Executes the external command. While it is running, the given runnable is executed.
-     * The external command is not checked until the runnable completes
-     *
-     * @param runnable A task to perform while the external command is running.
-     */
-    public void executeWhile(Runnable runnable) {
-        start();
-        if (runnable != null) {
-            runnable.run();
-        }
-        finish();
-    }
-
-    public String getCommandLine() {
-        StringBuilder sb = new StringBuilder();
-        for (String s : cmdArray) {
-            sb.append(s);
-            sb.append(" ");
-        }
-        return sb.toString();
-    }
-
-    /**
-     * Wait a given time for the process to finish
-     *
-     * @param maxWait the maximum amount of time in milliseconds to wait for the process to finish
-     *
-     * @return true if the process has finished.
-     */
-    public boolean finish(int maxWait) {
-        if (process != null) {
-            boolean finished = false;
-            try {
-                long endTime = System.currentTimeMillis() + maxWait;
-                awaitPump(outputPump, endTime);
-                awaitPump(inputPump, endTime);
-                awaitPump(errorPump, endTime);
-            } finally {
-                if (!arePumpsRunning()) {
-                    // process finished
-                    finished = true;
-                    int exitCode = wrapUpProcess();
-                    handler.complete(exitCode, processException);
-                }
-            }
-            return finished;
-        } else {
-            handler.complete(-1, processException);
-            return true;
-        }
-    }
-
-    private int wrapUpProcess() {
-        int exitCode = -1;
-        boolean processIncomplete = true;
-        try {
-            exitCode = process.exitValue();
-            processIncomplete = false;
-        } catch (IllegalThreadStateException e) {
-            // process still running - could be a race to have the process finish so wait a little to be sure
-            while (processIncomplete && System.currentTimeMillis() - getTimeoutTime() < 10) {
-                // we are currently before the end of the period (within 10ms slack), so process probably not ready yet
-                try {
-                    Thread.sleep(100);
-                    exitCode = process.exitValue();
-                    processIncomplete = false;
-                } catch (InterruptedException e1) {
-                    // just ignore
-                } catch (IllegalThreadStateException e2) {
-                    // ignore and try in the next loop
-                }
-            }
-        } finally {
-            process.destroy();
-        }
-
-        // make sure pumps are done
-        if (arePumpsRunning()) {
-            cancel();
-        }
-
-        if (processIncomplete) {
-            processException = new ProcessTimeoutException("process timed out");
-        }
-        return exitCode;
-    }
-
-    private void awaitPump(LatchedRunnable runnable, long latestTime) {
-        if (runnable != null) {
-            long timeout = latestTime - System.currentTimeMillis();
-            if (timeout < 1) {
-                timeout = 1;
-            }
-            runnable.await(timeout);
-        }
-    }
-
-    /**
-     * Cancel should be called if you wish to interrupt process execution.
-     */
-    public void cancel() {
-        this.cancelled = true;
-        if (outputPump != null) {
-            outputPump.cancel();
-        }
-
-        if (inputPump != null) {
-            inputPump.cancel();
-        }
-
-        if (errorPump != null) {
-            errorPump.cancel();
-        }
-        process.destroy();
-    }
-
-    public void setTimeout(long timeout) {
-        this.timeout = timeout;
-    }
-    
-    public static void shutdown() {
-        if (pumpThreadPool != null) {
-            pumpThreadPool.shutdown();
-        }
-    }
+    public String getCommandLine();
 }

processutils/src/main/java/com/atlassian/utils/process/ExternalProcessBuilder.java

     private Map<String, String> environment = new HashMap<String, String>();
     private File workingDir;
     private long timeout;
+    private static ExternalProcessFactory externalProcessFactory = new ExternalProcessFactory.Default();
 
     public ExternalProcessBuilder handlers(InputHandler input, OutputHandler output, OutputHandler error) {
         this.input = input;
         return this;
     }
     
-    public ExternalProcessBuilder env(Map<String, String> environment) {
-        environment.putAll(environment);
+    public ExternalProcessBuilder env(Map<String, String> env) {
+        environment.putAll(env);
         return this;
     }
     
             h = plugHandler;
         }
 
-        ExternalProcess process = new ExternalProcess(command, h);
-        if (timeout > 0L) {
-            process.setTimeout(timeout);
-        }
-        process.setWorkingDir(workingDir);
-        for (ProcessMonitor monitor: monitors) {
-            if (monitor != null) {
-                process.addMonitor(monitor);
-            }
-        }
-        if (!environment.isEmpty()) {
-            String[] env = new String[environment.size()];
-            int index = 0;
-            for (Entry<String, String> entry : environment.entrySet()) {
-                env[index++] = entry.getKey() + "=" + entry.getValue();
-            }
-            process.setEnvironment(env);
-        }
+        return externalProcessFactory.createExternalProcess(command, timeout, workingDir, monitors, environment, h);
+    }
 
-        return process;
+
+    public static void setExternalProcessFactory(ExternalProcessFactory factory) {
+        externalProcessFactory = factory;
     }
+
+    public static void resetExternalProcessFactory() {
+        externalProcessFactory = new ExternalProcessFactory.Default();
+    }
+
 }

processutils/src/main/java/com/atlassian/utils/process/ExternalProcessFactory.java

+package com.atlassian.utils.process;
+
+import java.io.File;
+import java.util.List;
+import java.util.Map;
+
+public interface ExternalProcessFactory {
+
+
+    ExternalProcess createExternalProcess(List<String> command, long timeout, File workingDir,
+                                          List<ProcessMonitor> monitors, Map<String,String> environment,
+                                          ProcessHandler handler);
+
+    void shutdown();
+
+    public static class Default implements ExternalProcessFactory {
+
+        public ExternalProcess createExternalProcess(List<String> command, long timeout, File workingDir,
+                                                     List<ProcessMonitor> monitors, Map<String,String> environment,
+                                                     ProcessHandler handler) {
+            ExternalProcessImpl process =  new ExternalProcessImpl(command, handler);
+            return configureProcess(process, timeout, workingDir, monitors, environment);
+        }
+
+        public void shutdown() {
+            ExternalProcessImpl.shutdown();
+        }
+
+        protected ExternalProcess configureProcess(ExternalProcessImpl process, long timeout, File workingDir,
+                                                   List<ProcessMonitor> monitors, Map<String, String> environment) {
+            if (timeout > 0L) {
+                process.setTimeout(timeout);
+            }
+            process.setWorkingDir(workingDir);
+            for (ProcessMonitor monitor: monitors) {
+                if (monitor != null) {
+                    process.addMonitor(monitor);
+                }
+            }
+            if (!environment.isEmpty()) {
+                String[] env = new String[environment.size()];
+                int index = 0;
+                for (Map.Entry<String, String> entry : environment.entrySet()) {
+                    env[index++] = entry.getKey() + "=" + entry.getValue();
+                }
+                process.setEnvironment(env);
+            }
+            return process;
+        }
+
+
+    }
+
+
+
+}

processutils/src/main/java/com/atlassian/utils/process/ExternalProcessImpl.java

+package com.atlassian.utils.process;
+
+import org.apache.log4j.Logger;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.SynchronousQueue;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * This class manages the execution of an external process, using separate threads to process
+ * the process' IO requirements.
+ */
+public class ExternalProcessImpl implements ExternalProcess {
+    private String[] cmdArray;
+    private File workingDir;
+    private String[] environment;
+    private ProcessHandler handler;
+    private Process process;
+
+    private List<ProcessMonitor> monitors = new ArrayList<ProcessMonitor>();
+
+    private ProcessException processException;
+
+    private LatchedRunnable outputPump;
+    private LatchedRunnable errorPump;
+    private LatchedRunnable inputPump;
+
+    private static final ExecutorService pumpThreadPool;
+
+    private long lastWatchdogReset;
+    private long timeout = 60000L;
+    private Long startTime;
+    private boolean cancelled;
+
+    public void resetWatchdog() {
+        lastWatchdogReset = System.currentTimeMillis();
+    }
+
+    public long getTimeoutTime() {
+        return lastWatchdogReset + timeout;
+    }
+
+    public boolean isTimedOut() {
+        return getTimeoutTime() < System.currentTimeMillis();
+    }
+
+    static {
+        ThreadFactory threadFactory = new ThreadFactory() {
+            public Thread newThread(Runnable r) {
+                return new Thread(r, "ExtProcess IO Pump");
+            }
+        };
+        pumpThreadPool = new ThreadPoolExecutor(6, Integer.MAX_VALUE, 120, TimeUnit.SECONDS,
+                                                new SynchronousQueue<Runnable>(), threadFactory);
+    }
+
+    /**
+     * Process an external command.
+     * @param cmdArray the command and its arguments as separate elements
+     * @param handler The handler for this execution. The handler supports the required IO
+     *                operations
+     */
+    public ExternalProcessImpl(String[] cmdArray, ProcessHandler handler) {
+        setCommand(cmdArray);
+        setHandler(handler);
+    }
+
+    /**
+     * Process an external command (the command and arguments are given as a list)
+     * @param command A list containing the command and its arguments
+     * @param handler The process handler to manage the execution of this process.
+     */
+    public ExternalProcessImpl(List<String> command, ProcessHandler handler) {
+        setCommand(command.toArray(new String[command.size()]));
+        setHandler(handler);
+    }
+
+    /**
+     * Process an external command. The command is given as a single command line and parsed into
+     * the command and its arguments. Spaces are used as argument delimiters so if any command arguments
+     * need to contain spaces, the array or list based constructors should be used.
+     *
+     * @param commandLine the command and its arguments in a single line. If any arguments
+     *                    need to contain spaces, the array or list based constructors should be used.
+     * @param handler The handler for this execution. The handler supports the required IO
+     *                operations
+     */
+    public ExternalProcessImpl(String commandLine, ProcessHandler handler) {
+        String[] cmdArray = ProcessUtils.tokenizeCommand(commandLine);
+        setCommand(cmdArray);
+        setHandler(handler);
+    }
+
+    private void setHandler(ProcessHandler handler) {
+        this.handler = handler;
+    }
+
+    private void setCommand(String[] cmdArray) {
+        this.cmdArray = cmdArray;
+    }
+
+    public void setWorkingDir(File workingDir) {
+        this.workingDir = workingDir;
+    }
+
+    public void setEnvironment(String[] environment) {
+        this.environment = environment;
+    }
+
+    private boolean arePumpsRunning() {
+        return outputPump.isRunning() || errorPump.isRunning()
+                || (inputPump != null && inputPump.isRunning());
+    }
+
+    /**
+     * Get the process handler for this process execution
+     *
+     * @return the ProcessHandler instance associated with this process execution.
+     */
+    public ProcessHandler getHandler() {
+        return handler;
+    }
+
+    /**
+     * @return the time process execution started. null if the process has not yet started.
+     */
+    public Long getStartTime() {
+        return this.startTime;
+    }
+    
+    public void addMonitor(ProcessMonitor monitor) {
+        this.monitors.add(monitor);
+    }
+    
+    public void removeMonitor(ProcessMonitor monitor) {
+        this.monitors.remove(monitor);
+    }
+    
+    private boolean isWindows() {
+    	return System.getProperty("os.name").toLowerCase().contains("windows");
+    }
+    
+    private String quoteString(String value) {
+        StringBuilder builder = new StringBuilder()
+            .append("\"")
+            .append(value.replace("\"", "\\\""))
+            .append("\"");
+        return builder.toString();
+    }
+    
+    /*
+      * This method provides a workaround for a JVM bug on windows (see http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4947220). The bug
+      * is that the Sun/Oracle JVM uses the (8 bit) OEM codepage for encoding commandline arguments that are passed to an external process.  Any
+      * characters in a command line argument that can't be represented in the OEM codepage will be replaced by the '?' character, which will probably
+      * cause the command that's being called to fail.
+      * 
+      * A bit of background information is helpful to understand what's going on. Windows uses 2 code pages: the OEM code page and the ANSI code page 
+      * (or 'Windows code page'). The OEM code page is always limited to 8 bit character encodings, whereas some windows code pages are 8 bit and some
+      * are larger. The OEM code page is typically used for console applications, whereas the windows code pages are used for native non-Unicode application
+      * using a GUI on Windows systems.  The system-wide settings can be found in the windows registry:
+      * 
+      * 
+      * More info about the history of OEM vs ANSI code pages: http://blogs.msdn.com/b/michkap/archive/2005/02/08/369197.aspx
+      * 
+      * The workaround is to store the command-line arguments in environment variables and refer to the environment vars on the command line. Windows
+      * cmd will expand the command line arguments without using to the OEM code page, which usually has more correlation with the locale of the 
+      * producer of the hardware (it's derived from the BIOS code page) than with the regional settings configured in Windows. The ANSI code page is derived from
+      * the regional settings.
+      * 
+      * When cmd expands the %JENV_XXX% vars on the command line it uses the ANSI code page instead of the OEM code page (or that is what testing
+      * seems to indicate, can't find any definitive answer in the cmd docos). While this still isn't a true fix to the problem, for most situations this will be sufficient 
+      * as the user typically won't use characters that aren't defined for his locale. But then again, they might..
+     */
+    private Process createWinProcess(String[] cmdArray, String[] environment, File workingDir) throws IOException {
+        Map<String, String> newEnv = new HashMap<String, String>();
+        if (environment == null) {
+            // inherit the environment of the current process
+            newEnv.putAll(System.getenv());
+        } else {
+            // add the env entries
+            for (String env: environment) {
+                String[] values = env.split("=");
+                if (values.length > 1) {
+                    newEnv.put(values[0], values[1]);
+                }
+            }
+        }
+        int extraArgs = 3;
+        String[] i18n = new String[cmdArray.length + extraArgs];
+        i18n[0] = "cmd";
+        i18n[1] = "/A"; // use ANSI encoding 
+        i18n[2] = "/C";
+        i18n[extraArgs] = cmdArray[0];
+        for (int counter = 1; counter < cmdArray.length; counter++) {
+            String envName = "JENV_" + counter;
+            i18n[counter + extraArgs] = "%" + envName + "%";
+            newEnv.put(envName, quoteString(cmdArray[counter]));
+        }
+        cmdArray = i18n;
+
+        ProcessBuilder pb = new ProcessBuilder(cmdArray);
+        pb.directory(workingDir);
+        Map<String, String> env = pb.environment();
+        env.putAll(newEnv);
+        return pb.start();
+    }
+
+    protected Process createProcess(String[] cmdArray, String[] environment, File workingDir) throws IOException {
+        if (isWindows()) {
+            return createWinProcess(cmdArray, environment, workingDir);
+        } else {
+            return Runtime.getRuntime().exec(cmdArray, environment, workingDir);
+        }
+    }
+
+    /**
+     * Start the external process and setup the IO pump threads needed to
+     * manage the process IO. If you call this method you must eventually call the
+     * finish() method. Using this method you may execute additional code between process
+     * start and finish.
+     */
+    public void start() {
+        try {
+            this.startTime = System.currentTimeMillis();
+            this.process = createProcess(cmdArray, environment, workingDir);
+            setupIOPumps();
+        } catch (IOException e) {
+            processException = new ProcessException(e);
+        }
+    }
+
+    private void setupIOPumps() {
+        // set up threads to feed data to and extract data from the process
+        if (handler.hasInput()) {
+            inputPump = new LatchedRunnable() {
+                protected void doTask() {
+                    handler.provideInput(process.getOutputStream());
+                }
+            };
+        }
+
+        errorPump = new LatchedRunnable() {
+            protected void doTask() {
+                try {
+                    handler.processError(process.getErrorStream());
+                } catch (Throwable e) {
+                    if (!isCancelled()) {
+                        processException = new ProcessException(e);
+                    }
+                }
+            }
+        };
+
+        outputPump = new LatchedRunnable() {
+            protected void doTask() {
+                try {
+                    handler.processOutput(process.getInputStream());
+                } catch (Throwable e) {
+                    if (!isCancelled()) {
+                        processException = new ProcessException(e);
+                    }
+                }
+            }
+        };
+
+        // tickle the dog initially
+        resetWatchdog();
+        handler.setWatchdog(this);
+
+        pumpThreadPool.execute(errorPump);
+        pumpThreadPool.execute(outputPump);
+        if (inputPump != null) {
+            pumpThreadPool.execute(inputPump);
+        }
+    }
+
+    /**
+     * Finish process execution. This method should be called after you have called the
+     * start() method.
+     */
+    public void finish() {
+        if (process != null) {
+            try {
+                do {
+                    long checkTime = getTimeoutTime();
+                    awaitPump(outputPump, checkTime);
+                    awaitPump(inputPump, checkTime);
+                    awaitPump(errorPump, checkTime);
+                } while (!isTimedOut() && arePumpsRunning());
+            } finally {
+                int exitCode = 0;
+                if (!cancelled) {
+                    exitCode = wrapUpProcess();
+                }
+                handler.complete(exitCode, processException);
+            }
+        } else {
+            handler.complete(-1, processException);
+        }
+    }
+    
+    /**
+     * Notifies all ProcessMonitors of the 'beforeStart' event.
+     */
+    private void notifyBeforeStart() {
+        for (ProcessMonitor monitor: monitors) {
+            try {
+                monitor.onBeforeStart(this);
+            } catch (Exception e) {
+                // catch and log error, but continue
+                Logger.getLogger(ExternalProcessImpl.class).error("Error while processing 'beforeStarted' event:", e);
+            }
+        }
+    }
+
+    /**
+     * Notifies all ProcessMonitors of the 'afterFinished' event.
+     */
+    private void notifyAfterFinished() {
+        for (ProcessMonitor monitor: monitors) {
+            try {
+                monitor.onAfterFinished(this);
+            } catch (Exception e) {
+                Logger.getLogger(ExternalProcessImpl.class).error("Error while processing 'afterFinished' event:", e);
+            }
+        }
+    }
+    
+    /**
+     * Execute the external command. When this method returns, the process handler
+     * provided at construction time should be consulted to collect exit code, exceptions,
+     * process output, etc.
+     */
+    public void execute() {
+        notifyBeforeStart();
+        try {
+            start();
+            finish();
+        } finally {
+            notifyAfterFinished();
+        }
+    }
+
+    /**
+     * Executes the external command. While it is running, the given runnable is executed.
+     * The external command is not checked until the runnable completes
+     *
+     * @param runnable A task to perform while the external command is running.
+     */
+    public void executeWhile(Runnable runnable) {
+        start();
+        if (runnable != null) {
+            runnable.run();
+        }
+        finish();
+    }
+
+    public String getCommandLine() {
+        StringBuilder sb = new StringBuilder();
+        for (String s : cmdArray) {
+            sb.append(s);
+            sb.append(" ");
+        }
+        return sb.toString();
+    }
+
+    /**
+     * Wait a given time for the process to finish
+     *
+     * @param maxWait the maximum amount of time in milliseconds to wait for the process to finish
+     *
+     * @return true if the process has finished.
+     */
+    public boolean finish(int maxWait) {
+        if (process != null) {
+            boolean finished = false;
+            try {
+                long endTime = System.currentTimeMillis() + maxWait;
+                awaitPump(outputPump, endTime);
+                awaitPump(inputPump, endTime);
+                awaitPump(errorPump, endTime);
+            } finally {
+                if (!arePumpsRunning()) {
+                    // process finished
+                    finished = true;
+                    int exitCode = wrapUpProcess();
+                    handler.complete(exitCode, processException);
+                }
+            }
+            return finished;
+        } else {
+            handler.complete(-1, processException);
+            return true;
+        }
+    }
+
+    private int wrapUpProcess() {
+        int exitCode = -1;
+        boolean processIncomplete = true;
+        try {
+            exitCode = process.exitValue();
+            processIncomplete = false;
+        } catch (IllegalThreadStateException e) {
+            // process still running - could be a race to have the process finish so wait a little to be sure
+            while (processIncomplete && System.currentTimeMillis() - getTimeoutTime() < 10) {
+                // we are currently before the end of the period (within 10ms slack), so process probably not ready yet
+                try {
+                    Thread.sleep(100);
+                    exitCode = process.exitValue();
+                    processIncomplete = false;
+                } catch (InterruptedException e1) {
+                    // just ignore
+                } catch (IllegalThreadStateException e2) {
+                    // ignore and try in the next loop
+                }
+            }
+        } finally {
+            process.destroy();
+        }
+
+        // make sure pumps are done
+        if (arePumpsRunning()) {
+            cancel();
+        }
+
+        if (processIncomplete) {
+            processException = new ProcessTimeoutException("process timed out");
+        }
+        return exitCode;
+    }
+
+    private void awaitPump(LatchedRunnable runnable, long latestTime) {
+        if (runnable != null) {
+            long timeout = latestTime - System.currentTimeMillis();
+            if (timeout < 1) {
+                timeout = 1;
+            }
+            runnable.await(timeout);
+        }
+    }
+
+    /**
+     * Cancel should be called if you wish to interrupt process execution.
+     */
+    public void cancel() {
+        this.cancelled = true;
+        if (outputPump != null) {
+            outputPump.cancel();
+        }
+
+        if (inputPump != null) {
+            inputPump.cancel();
+        }
+
+        if (errorPump != null) {
+            errorPump.cancel();
+        }
+        process.destroy();
+    }
+
+    public void setTimeout(long timeout) {
+        this.timeout = timeout;
+    }
+    
+    public static void shutdown() {
+        if (pumpThreadPool != null) {
+            pumpThreadPool.shutdown();
+        }
+    }
+}

processutils/src/test/java/com/atlassian/utils/process/CallEcho.java

 
 /**
  * Utility class that calls 'echo'. This is implemented as a separate java application (with a main) to allow it to be called
- * with different -Dfile.encoding settings. See {@link ExternalProcessTest} for more info. 
+ * with different -Dfile.encoding settings. See {@link ProcessBuilderEncodingTest} for more info.
  */
 public class CallEcho {
 
                         protected void processLine(int lineNum, String line) {
                             System.err.println(line);
                         }
-                    }).build();
-            process.setEnvironment(new String[] { "CLASSPATH=" + System.getProperty("java.class.path") });
+                    }).env("CLASSPATH", System.getProperty("java.class.path")).build();
+
             process.execute();
 
             // output the expected and actual strings as a list of bytes to
             System.out.println(getBytes(echo));
             System.out.println(getBytes(result[0]));
         }
-        ExternalProcess.shutdown();
+        ExternalProcessImpl.shutdown();
     }
 }

processutils/src/test/java/com/atlassian/utils/process/ExternalProcessTest.java

-package com.atlassian.utils.process;
-
-import static org.junit.Assert.*;
-
-import java.io.BufferedReader;
-import java.io.InputStreamReader;
-import java.nio.charset.Charset;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-
-import org.junit.Test;
-
-public class ExternalProcessTest {
-    private static class EchoResult {
-        public String input;
-        public String output;
-    }
-    
-    /*
-     * Starts the CallEcho as an separate process. This is necessary because the jvm uses the default encoding (-Dfile.encoding) for encoding commandline
-     * arguments that are provided to external processes. This test tests whether encodings that should be compatible are actually compatible. This is
-     * needed for http://jira.atlassian.com/browse/CRUC-4793 which provides a workaround for a JVM bug on windows.
-     */
-    protected EchoResult spawnEcho(String encoding, String... testStrings) throws Exception {
-        List<String> cmd = new ArrayList<String>(Arrays.asList("java", "-Dfile.encoding=" + encoding, CallEcho.class.getName()));
-        cmd.addAll(Arrays.asList(testStrings));
-        
-        ProcessBuilder processBuilder = new ProcessBuilder(cmd);
-        processBuilder.environment().put("CLASSPATH", System.getProperty("java.class.path"));
-        Process process = processBuilder.start();
-        
-        BufferedReader reader = null;
-        try {
-            reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
-            EchoResult result = new EchoResult();
-            result.input = reader.readLine();
-            result.output = reader.readLine();
-            
-            return result;
-        } finally {
-            reader.close();
-        }
-    }
-    
-    protected void assertNotEquals(String message, String value1, String value2) {
-        assertFalse(message + "; result was : " + value1, (value1 == null && value2 == null) || (value1 != null && value1.equals(value2)));
-    }
-    
-    protected boolean isWindows() {
-        return System.getProperty("os.name").toLowerCase().contains("windows");
-    }
-    
-    @Test
-    public void testIncompatibleCommandLineArgumentsASCII() throws Exception {
-        EchoResult result = spawnEcho("ASCII", "iso-8859-1");
-        assertNotEquals("ASCII - incompatibility expected", result.input, result.output);
-
-        result = spawnEcho("ASCII", "unicode-chinese");
-        assertNotEquals("ASCII - incompatibility expected", result.input, result.output);
-}
-
-    /**
-     * This test will only succeed on windows if the ANSI code page that is defined is compatible with iso-8859-1 (e.g. cp1252)
-     * @see ExternalProcess#createWinProcess for more details
-     */
-    @Test
-    public void testCommandLineArgumentsUTF8() throws Exception {
-        EchoResult result = spawnEcho("UTF-8",  "iso-8859-1");
-        assertEquals("UTF-8 - incompatibility detected:", result.input, result.output);
-    }
-    
-    /**
-     * This test is expected to fail on windows (assuming the ANSI CP is cp1252) because on windows that code page
-     * is used to encode the commandline arguments.
-     */
-    @Test
-    public void testCommandLineArgumentsUTF8IncompatibleWithCp1252() throws Exception {
-        EchoResult result = spawnEcho("UTF-8",  "unicode-chinese");
-        
-        if (isWindows()) {
-        	assertNotEquals("UTF-8 - incompatibility expected", result.input, result.output);
-        } else {
-        	assertEquals("UTF-8 - incompatibility detected", result.input, result.output);
-        }
-    }
-    
-    @Test
-    public void testCommandLineArgumentsWin1252() throws Exception {
-        if (Charset.isSupported("windows-1252")) {
-            EchoResult result = spawnEcho("windows-1252", "iso-8859-1");
-            assertEquals("Windows cp 1252 -  incompatibility detected:", result.input, result.output);
-        }
-    }
-
-    @Test
-    public void testCommandLineArgumentsWin1252Iso88591Incompatibilities() throws Exception {
-        if (Charset.isSupported("windows-1252") && isWindows()) {
-            // other platforms claim to support windows-1252, but it looks like only iso-8859-1 is supported.
-            EchoResult result = spawnEcho("windows-1252", "win-1252-not-in-iso-8859-1");
-            assertEquals("Windows cp 1252 -  incompatibility detected:", result.input, result.output);
-        }
-    }
-
-    @Test
-    public void testCommandLineArgumentsWin437() throws Exception {
-        if (Charset.isSupported("windows-437")) {
-            EchoResult result = spawnEcho("windows-437", "windows-437");
-            assertEquals("Windows cp 437 - incompatibility detected:", result.input, result.output);
-        }
-    }
-    
-    @Test
-    public void testCommandLineArgumentsIso88591() throws Exception {
-        EchoResult result = spawnEcho("iso-8859-1", "iso-8859-1");
-        assertEquals("ISO-8859-1 - incompatibility detected:", result.input, result.output);
-    }
-}

processutils/src/test/java/com/atlassian/utils/process/ProcessBuilderEncodingTest.java

+package com.atlassian.utils.process;
+
+import static org.junit.Assert.*;
+
+import java.io.BufferedReader;
+import java.io.InputStreamReader;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.Test;
+
+public class ProcessBuilderEncodingTest {
+    private static class EchoResult {
+        public String input;
+        public String output;
+    }
+    
+    /*
+     * Starts the CallEcho as an separate process. This is necessary because the jvm uses the default encoding (-Dfile.encoding) for encoding commandline
+     * arguments that are provided to external processes. This test tests whether encodings that should be compatible are actually compatible. This is
+     * needed for http://jira.atlassian.com/browse/CRUC-4793 which provides a workaround for a JVM bug on windows.
+     */
+    protected EchoResult spawnEcho(String encoding, String... testStrings) throws Exception {
+        List<String> cmd = new ArrayList<String>(Arrays.asList("java", "-Dfile.encoding=" + encoding, CallEcho.class.getName()));
+        cmd.addAll(Arrays.asList(testStrings));
+        
+        ProcessBuilder processBuilder = new ProcessBuilder(cmd);
+        processBuilder.environment().put("CLASSPATH", System.getProperty("java.class.path"));
+        Process process = processBuilder.start();
+        
+        BufferedReader reader = null;
+        try {
+            reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
+            EchoResult result = new EchoResult();
+            result.input = reader.readLine();
+            result.output = reader.readLine();
+            
+            return result;
+        } finally {
+            reader.close();
+        }
+    }
+    
+    protected void assertNotEquals(String message, String value1, String value2) {
+        assertFalse(message + "; result was : " + value1, (value1 == null && value2 == null) || (value1 != null && value1.equals(value2)));
+    }
+    
+    protected boolean isWindows() {
+        return System.getProperty("os.name").toLowerCase().contains("windows");
+    }
+    
+    @Test
+    public void testIncompatibleCommandLineArgumentsASCII() throws Exception {
+        EchoResult result = spawnEcho("ASCII", "iso-8859-1");
+        assertNotEquals("ASCII - incompatibility expected", result.input, result.output);
+
+        result = spawnEcho("ASCII", "unicode-chinese");
+        assertNotEquals("ASCII - incompatibility expected", result.input, result.output);
+}
+
+    /**
+     * This test will only succeed on windows if the ANSI code page that is defined is compatible with iso-8859-1 (e.g. cp1252)
+     * @see ExternalProcess#createWinProcess for more details
+     */
+    @Test
+    public void testCommandLineArgumentsUTF8() throws Exception {
+        EchoResult result = spawnEcho("UTF-8",  "iso-8859-1");
+        assertEquals("UTF-8 - incompatibility detected:", result.input, result.output);
+    }
+    
+    /**
+     * This test is expected to fail on windows (assuming the ANSI CP is cp1252) because on windows that code page
+     * is used to encode the commandline arguments.
+     */
+    @Test
+    public void testCommandLineArgumentsUTF8IncompatibleWithCp1252() throws Exception {
+        EchoResult result = spawnEcho("UTF-8",  "unicode-chinese");
+
+        if (isWindows()) {
+            assertNotEquals("UTF-8 - incompatibility expected", result.input, result.output);
+        } else {
+            assertEquals("UTF-8 - incompatibility detected", result.input, result.output);
+        }
+    }
+    
+    @Test
+    public void testCommandLineArgumentsWin1252() throws Exception {
+        if (Charset.isSupported("windows-1252")) {
+            EchoResult result = spawnEcho("windows-1252", "iso-8859-1");
+            assertEquals("Windows cp 1252 -  incompatibility detected:", result.input, result.output);
+        }
+    }
+
+    @Test
+    public void testCommandLineArgumentsWin1252Iso88591Incompatibilities() throws Exception {
+        if (Charset.isSupported("windows-1252") && isWindows()) {
+            // other platforms claim to support windows-1252, but it looks like only iso-8859-1 is supported.
+            EchoResult result = spawnEcho("windows-1252", "win-1252-not-in-iso-8859-1");
+            assertEquals("Windows cp 1252 -  incompatibility detected:", result.input, result.output);
+        }
+    }
+
+    @Test
+    public void testCommandLineArgumentsWin437() throws Exception {
+        if (Charset.isSupported("windows-437")) {
+            EchoResult result = spawnEcho("windows-437", "windows-437");
+            assertEquals("Windows cp 437 - incompatibility detected:", result.input, result.output);
+        }
+    }
+    
+    @Test
+    public void testCommandLineArgumentsIso88591() throws Exception {
+        EchoResult result = spawnEcho("iso-8859-1", "iso-8859-1");
+        assertEquals("ISO-8859-1 - incompatibility detected:", result.input, result.output);
+    }
+}