Commits

Bryan Turner committed ec39f4f

STASHDEV-1957 Upgraded to winp-1.17-atlassian1.

- No longer requires the exclusion on junit:junit; the pom.xml for
winp has been fixed to move the dependency to test scope
- Pass the exit code from the process to internalCancel and only
use WinProcess.killRecursively() for non-zero exit codes
- Moved ProcessBuilderEncodingTest.testNativeKilling() into the
ExternalProcessImplTest and fix it so it runs correctly
- Fixed the blocking InputHandler tests on Windows (need to flush
the stream to get the "key" input to go to the process)

Comments (0)

Files changed (4)

         <dependency>
             <groupId>org.jvnet.winp</groupId>
             <artifactId>winp</artifactId>
-            <version>1.15-atlassian-1</version>
-            <exclusions>
-                <exclusion>
-                    <groupId>junit</groupId>
-                    <artifactId>junit</artifactId>
-                </exclusion>
-            </exclusions>	    
+            <version>1.17-atlassian1</version>
         </dependency>
         <dependency>
             <groupId>log4j</groupId>

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

                 }
             }
         } finally {
-            internalCancel();
+            internalCancel(exitCode);
         }
 
         if (processIncomplete && !interrupted) {
      */
     public void cancel() {
         if (canceled.compareAndSet(false, true)) {
-            internalCancel();
+            internalCancel(1);
         }
     }
 
-    private void internalCancel() {
-        
+    private void internalCancel(int exitCode) {
         if (inputPump != null) {
             // we determine whether we're going to interrupt the inputPump *after* the process has already finished.
             // if that is the case, we don't treat the InterruptedException that we catch from the inputPump as an error
         }
 
         if (process != null) {
-            if (isWindows()) {
+            if (isWindows() && exitCode != 0) {
                 try {
                     new WinProcess(process).killRecursively();
-                } catch (Throwable t) {
-                    LOG.error("Failed to kill Windows process; falling back on Process.destroy()", t);
+                } catch (Exception e) {
+                    LOG.error("Failed to kill Windows process; falling back on Process.destroy()", e);
                     process.destroy();
                 }
             } else {

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

                 try {
                     // this is for windows pause. Wait 1 second then press a key
                     input.write("key".getBytes());
+                    input.flush();
 
                     // sleep for a long time, the process should finish before the inputHandler finishes
                     // this simulates an inputHandler that simply copies data from some other stream into
         };
 
         //Ensure the timeout here is less than the timeout on the test
-        ExternalProcess process = builder.idleTimeout(TimeUnit.SECONDS.toMillis(15))
+        return builder.idleTimeout(TimeUnit.SECONDS.toMillis(15))
                 .handlers(inputHandler, new StringOutputHandler())
                 .build();
-        
-        return process;
     }
 
     @Test(timeout = 5 * 1000) // Test timeout in case execution doesn't stop as it should
         assertEquals(0, process.getHandler().getExitCode());
     }
 
+    @Test
+    public void testNativeWindowsKilling() {
+        if (isWindows()) {
+            ExternalProcess process = new ExternalProcessBuilder()
+                    .command(Arrays.asList("pause"))
+                    .handlers(new StringOutputHandler())
+                    .build();
+            process.start();
+            process.cancel();
+        }
+    }
+
     private boolean isWindows() {
         return System.getProperty("os.name").toLowerCase().contains("windows");
     }

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

             assertEquals("ISO-8859-1 - incompatibility detected:", result.input, result.output);
         }
     }
-
-    @Test
-    public void testNativeWindowsKilling() {
-        if (isWindows()) {
-            ExternalProcess process = new ExternalProcessBuilder().command(Arrays.asList("pause")).build();
-            process.start();
-            process.cancel();
-        }
-    }
 }
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.