Commits

Mark Chaimungkalanont  committed 488d25a

BAM-8650 Changes to the interrupt logic to ensure that the tasks and streams are destroyed when we interuppt the running thread.

  • Participants
  • Parent commits 15c3ab8

Comments (0)

Files changed (2)

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

 package com.atlassian.utils.process;
 
+import org.apache.log4j.Logger;
+import org.jvnet.winp.WinProcess;
+
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 
-import org.apache.log4j.Logger;
-import org.jvnet.winp.WinProcess;
-
 /**
  * This class manages the execution of an external process, using separate threads to process
  * the process' IO requirements.
     private void setupIOPumps() {
         // set up threads to feed data to and extract data from the process
         if (handler.hasInput()) {
-            inputPump = new LatchedRunnable() {
+            inputPump = new LatchedRunnable("inputPump") {
                 protected void doTask() {
                     handler.provideInput(process.getOutputStream());
                 }
             };
         }
 
-        errorPump = new LatchedRunnable() {
+        errorPump = new LatchedRunnable("errorPump") {
             protected void doTask() {
                 try {
                     handler.processError(process.getErrorStream());
                 } catch (Throwable e) {
                     if (!isCancelled()) {
+                        log.debug(name + ": Process wasn't cancelled, storing exception", e);
                         processException = new ProcessException(e);
                     }
+                    else {
+                        log.debug(name + ": Process cancelled ignoring exception", e);
+                    }
                 }
             }
         };
 
-        outputPump = new LatchedRunnable() {
+        outputPump = new LatchedRunnable("outputPump") {
             protected void doTask() {
                 try {
                     handler.processOutput(process.getInputStream());
                 } catch (Throwable e) {
                     if (!isCancelled()) {
+                        log.debug(name + ": Process wasn't cancelled, storing exception", e);
                         processException = new ProcessException(e);
                     }
+                    else {
+                        log.debug(name + ": Process cancelled ignoring exception", e);
+                    }
                 }
             }
         };
                     awaitPump(inputPump, checkTime);
                     awaitPump(errorPump, checkTime);
                 } while (!isTimedOut() && arePumpsRunning() && !Thread.currentThread().isInterrupted());
+            } finally {
 
-                if (Thread.currentThread().isInterrupted() && arePumpsRunning()) {
+                if (Thread.currentThread().isInterrupted()) {
                     cancel();
+
+                    // All is good, now clearing interrupted state of current thread.
+                    Thread.interrupted();
                 }
-            } finally {
-                int exitCode = 0;
-                if (!cancelled) {
-                    exitCode = wrapUpProcess();
-                }
+
+                int exitCode  = wrapUpProcess();
                 handler.complete(exitCode, processException);
             }
         } else {

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() {