Commits

Michael Heemskerk committed 9ae46db

More fixes to ensure that a process is not marked as failed when we intentionally interrupt the input IO pump after the process has already finished

Comments (0)

Files changed (2)

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

 
 import java.io.File;
 import java.io.IOException;
+import java.io.InterruptedIOException;
 import java.util.*;
 import java.util.concurrent.*;
 
     private long startTime;
     private File workingDir;
     private boolean useWindowsEncodingWorkaround;
+    private volatile boolean stdInPumpInterrupted = false;
 
     static {
         final String pooledThreadName = "ExtProcess IO Pump";
 
     private void handleHandlerError(String handlerName, Throwable t) {
         if (!isCanceled()) {
-            LOG.debug(handlerName + " encountered an error; aborting process", t);
             if (isAlive()) {
                 cancel();
             }
+            LOG.debug(handlerName + " encountered an error; aborting process", t);
             if (t instanceof ProcessException) {
                 processException = (ProcessException) t;
             } else {
                     try {
                         handler.provideInput(process.getOutputStream());
                     } catch (Throwable t) {
-                        handleHandlerError(name, t);
+                        // The StdInHandler IO pump is intentionally interrupted when the process terminates, the output and the
+                        // error pumps have finished and the StdInHandler does not finish by itself. In this case we do not
+                        // consider this to be an error condition and we ignore the exception
+                        if (!isStdInPumpInterrupted(t)) {
+                            handleHandlerError(name, t);
+                        }
                     }
                 }
             };
         }
     }
 
+    private boolean isStdInPumpInterrupted(Throwable t) {
+        if (stdInPumpInterrupted) {
+            while (t != null) {
+                if (t instanceof InterruptedException || t instanceof InterruptedIOException) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+    
     private int wrapUpProcess() {
         int exitCode = -1;
         boolean processIncomplete = true;
     }
 
     private void internalCancel() {
+        stdInPumpInterrupted = !isAlive() && inputPump.isRunning();
+
+        if (inputPump != null) {
+            inputPump.cancel();
+        }
         if (outputPump != null) {
             outputPump.cancel();
         }
+        if (errorPump != null) {
+            errorPump.cancel();
+        }
         if (inputPump != null) {
             inputPump.cancel();
         }
-        if (errorPump != null) {
-            errorPump.cancel();
-        }
         if (process != null) {
             if (isWindows()) {
                 try {

src/test/java/com/atlassian/utils/process/ExternalProcessImplTest.java

 import java.util.Arrays;
 import java.util.concurrent.TimeUnit;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 public class ExternalProcessImplTest {
 
         
         // the process should finish by itself and should not be marked as canceled.
         assertFalse(process.isCanceled());
+        assertTrue(process.getHandler().succeeded());
+        assertNull(process.getHandler().getException());
+        assertEquals(0, process.getHandler().getExitCode());
     }
 
     @Test(timeout = 5 * 1000) // Test timeout in case execution doesn't stop as it should
-    public void testBlockingInputHandlerAbortsWhenProcessFinished2() {
+    public void testBlockingInputHandlerAbortsWhenProcessFinishes2() {
         ExternalProcess process = buildProcessWithBlockingInputHandler();
 
         process.execute();
 
         // the process should finish by itself and should not be marked as canceled.
         assertTrue(process.finish(10 * 1000));
+        
         assertFalse(process.isCanceled());
+        assertTrue(process.getHandler().succeeded());
+        assertNull(process.getHandler().getException());
+        assertEquals(0, process.getHandler().getExitCode());
     }
-    
 
     private boolean isWindows() {
         return System.getProperty("os.name").toLowerCase().contains("windows");
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.