Commits

Bryan Turner committed 73f2dba Merge

Merging i386/process-utils into atlassian/process-utils for the 1.5.x branch

  • Participants
  • Parent commits 796c3d0, 689a21b
  • Branches 1.5.x

Comments (0)

Files changed (6)

File .hgignore

File contents unchanged.
 f583ef26fc4ceeefa968683749fa55f22afc59d2 atlassian-processutils-1.5-beta11
 c11b314b02f1c1490024dbc87623cfa0fc5f9e51 atlassian-processutils-1.5-rc1
 0006b2ff8f692eef47b9f5b149ddf02280d8abb6 atlassian-processutils-1.5-rc2
-788d7eacfc99bc2994594fbb5c68b98b6971bb60 atlassian-processutils-1.5-rc3
+5c95a4190871d8cfbc7bf61991bb0b8751288513 atlassian-processutils-1.5-rc3
+323e97095d3a837844b5c8037f1ef158e965d8d8 atlassian-processutils-1.5-rc4
+e9d64cf7f65991c0fee369b37f0268d888d6d025 atlassian-processutils-1.5-rc5
 9a018567077cff394c9985146613db5b675d7240 atlassian-processutils-1.5-rc5.stash
-3b4642858f31be4d6ee1fcab5064dbe6a05393a5 atlassian-processutils-1.5-rc6.stash
         <dependency>
             <groupId>org.jvnet.winp</groupId>
             <artifactId>winp</artifactId>
-            <version>1.14-atlassian-1</version>
+            <version>1.15-atlassian-1</version>
         </dependency>
         <dependency>
             <groupId>log4j</groupId>

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

File contents unchanged.

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

             newCommand.addAll(command);
         }
 
-        ProcessBuilder pb = new ProcessBuilder(newCommand)
+        ProcessBuilder builder = new ProcessBuilder(newCommand)
                 .directory(workingDir);
         if (environment != null) {
-            pb.environment().putAll(environment);
+            builder.environment().putAll(environment);
         }
-        return pb.start();
+        if (LOG.isDebugEnabled()) {
+            logProcessDetails(builder);
+        }
+        return builder.start();
     }
 
-    protected Process createProcess(List<String> cmdArray, Map<String, String> environment, File workingDir)
+    protected Process createProcess(List<String> command, Map<String, String> environment, File workingDir)
             throws IOException {
         if (!suppressSpecialWindowsBehaviour && isWindows()) {
-            return createWinProcess(cmdArray, environment, workingDir);
+            return createWinProcess(command, environment, workingDir);
         } else {
-            ProcessBuilder builder = new ProcessBuilder(cmdArray).directory(workingDir);
+            ProcessBuilder builder = new ProcessBuilder(command).directory(workingDir);
             if (environment != null) {
                 builder.environment().putAll(environment);
             }
+            if (LOG.isDebugEnabled()) {
+                logProcessDetails(builder);
+            }
             return builder.start();
         }
     }
 
+    private void logProcessDetails(ProcessBuilder processBuilder) {
+        String divider = "---------------------------";
+        LOG.debug(divider);
+        LOG.debug("Start Process Debug Information");
+        LOG.debug(divider);
+        LOG.debug("Command");
+        LOG.debug(processBuilder.command());
+        LOG.debug(divider);
+        LOG.debug("Working Dir");
+        LOG.debug(processBuilder.directory());
+        LOG.debug(divider);
+        LOG.debug("Environment");
+        for (Map.Entry entry : processBuilder.environment().entrySet()) {
+            LOG.debug(entry.getKey() + ": " + entry.getValue());
+        }
+        LOG.debug(divider);
+        LOG.debug("Redirect Error Stream?");
+        LOG.debug(processBuilder.redirectErrorStream());
+        LOG.debug(divider);
+        LOG.debug("End Process Debug Information");
+        LOG.debug(divider);
+    }
+
     /**
      * 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
     private void setupIOPumps() {
         // set up threads to feed data to and extract data from the process
         if (handler.hasInput()) {
-            inputPump = new LatchedRunnable() {
-                @Override
-                public String getName() {
-                    return "StdInHandler " + process;
-                }
+            inputPump = new LatchedRunnable("StdInHandler " + process) {
 
                 protected void doTask() {
                     handler.provideInput(process.getOutputStream());
             };
         }
 
-        errorPump = new LatchedRunnable() {
-            @Override
-            public String getName() {
-                return "StdErrHandler " + process;
-            }
+        errorPump = new LatchedRunnable("StdErrHandler " + process) {
 
             protected void doTask() {
                 try {
                     handler.processError(process.getErrorStream());
                 } catch (Throwable e) {
                     if (!isCancelled()) {
+                        LOG.debug(name + ": Process wasn't canceled; storing exception", e);
                         processException = new ProcessException(e);
+                    } else {
+                        LOG.debug(name + ": Process canceled; ignoring exception", e);
                     }
                 }
             }
         };
 
-        outputPump = new LatchedRunnable() {
-            @Override
-            public String getName() {
-                return "StdOutHandler " + process;
-            }
+        outputPump = new LatchedRunnable("StdOutHandler " + process) {
 
             protected void doTask() {
                 try {
                     handler.processOutput(process.getInputStream());
                 } catch (Throwable e) {
                     if (!isCancelled()) {
+                        LOG.debug(name + ": Process wasn't canceled; storing exception", e);
                         processException = new ProcessException(e);
+                    } else {
+                        LOG.debug(name + ": Process canceled; ignoring exception", e);
                     }
                 }
             }
                     awaitPump(outputPump, checkTime);
                     awaitPump(inputPump, checkTime);
                     awaitPump(errorPump, checkTime);
-                } while (!isTimedOut() && arePumpsRunning());
+                } while (!isTimedOut() && arePumpsRunning() && !Thread.currentThread().isInterrupted());
             } finally {
-                int exitCode = 0;
-                if (!canceled) {
-                    exitCode = wrapUpProcess();
+                if (Thread.currentThread().isInterrupted()) {
+                    cancel();
+
+                    // All is good, now clearing interrupted state of current thread.
+                    Thread.interrupted();
                 }
+
+                int exitCode = wrapUpProcess();
                 handler.complete(exitCode, processException);
             }
         } else {
                     exitCode = process.exitValue();
                     processIncomplete = false;
                 } catch (InterruptedException ie) {
+                    processIncomplete = true;
                     interrupted = true;
                     break;
                 } catch (IllegalThreadStateException e) {
                 }
             }
         } finally {
-            process.destroy();
-        }
-
-        // make sure pumps are done
-        if (arePumpsRunning()) {
             cancel();
         }
 
-        if (interrupted) {
-            processException = new ProcessException("Process interrupted");
-        } else if (processIncomplete) {
+        if (processIncomplete && !interrupted) {
             processException = new ProcessTimeoutException("process timed out");
         }
         return exitCode;
         if (outputPump != null) {
             outputPump.cancel();
         }
-
         if (inputPump != null) {
             inputPump.cancel();
         }
-
         if (errorPump != null) {
             errorPump.cancel();
         }
-
-        if (isWindows()) {
-            try {
-                new WinProcess(process).killRecursively();
-            } catch (Throwable t) {
-                LOG.error("Failed to kill Windows process; falling back on Process.destroy()", t);
+        if (process != null) {
+            if (isWindows()) {
+                try {
+                    new WinProcess(process).killRecursively();
+                } catch (Throwable t) {
+                    LOG.error("Failed to kill Windows process; falling back on Process.destroy()", t);
+                    process.destroy();
+                }
+            } else {
                 process.destroy();
             }
-        } else {
-            process.destroy();
         }
     }
 

File src/main/java/com/atlassian/utils/process/LatchedRunnable.java

     private Thread runner;
     private boolean cancelled;
     private Stack<?> ndcStack = NDC.cloneStack();
+    protected String name;
+
+    protected LatchedRunnable()
+    {
+    }
+
+    protected LatchedRunnable(final String name)
+    {
+        this.name = name;
+    }
 
     public final void run() {
         try {
             return latch.await(millis, TimeUnit.MILLISECONDS);
         } catch (InterruptedException e) {
             log.warn("Interrupted waiting for ExternalProcess pump to complete");
-            throw new RuntimeException("Interrupted waiting for Process pump", e);
+            Thread.currentThread().interrupt();
+            return false;
         }
     }
 
 
     public void cancel() {
         this.cancelled = true;
+
+        // Generally pointless, giving likely underlying blocking io, but logically correct in any case. Technically, doTask could do anything
+        interrupt();
     }
 
     public boolean isCancelled() {