Changeset 31642

Timestamp:

2017-05-02T19:41:53+12:00 (7 years ago)

Author:

ak19

Message:

For Windows, process.destroy() doesn't get called on interruption because the join() calls complete and process is set to null, so not triggering process.destroy(). Now the code explicitly triggers process.destroy() on cancel/interruptedException and on any Exception, by means of new member forciblyTerminateProcess. And the code no longer sets process to null until the very end of finishing up with the process.

File:

• main/trunk/gli/src/org/greenstone/gatherer/util/SafeProcess.java (modified) (16 diffs)

main/trunk/gli/src/org/greenstone/gatherer/util/SafeProcess.java

@@ -39 +39 @@
     private String inputStr = null;
     private Process process = null;
-
+    private boolean forciblyTerminateProcess = false;
+    
     // output from running SafeProcess.runProcess()
     private String outputStr = ""; 
@@ -138 +139 @@
                    SafeProcess.InputStreamGobbler errorGobbler)
     throws IOException, InterruptedException
-    {
-
-    
+    {   
     // kick off the stream gobblers
     inputGobbler.start();
@@ -176 +175 @@
         errorGobbler.interrupt();
         outputGobbler.interrupt();
-
+        
+        // Since we have been cancelled (InterruptedException), or on any Exception, we need
+        // to forcibly terminate process eventually after the finally code first waits for each worker thread
+        // to die off. Don't set process=null until after we've forcibly terminated it if needs be.
+        this.forciblyTerminateProcess = true; 
+        
         // even after the interrupts, we want to proceed to calling join() on all the worker threads
         // in order to wait for each of them to die before attempting to destroy the process if it
@@ -204 +208 @@
         // set the variables that the code which created a SafeProcess object may want to inspect
         this.outputStr = outputGobbler.getOutput();
-        this.errorStr = errorGobbler.getOutput();
-        
-        // Since we didn't have an exception, process should have terminated now (waitFor blocks until then)
-        // Set process to null so we don't forcibly terminate it below with process.destroy()
-        this.process = null;        
+        this.errorStr = errorGobbler.getOutput();       
     }
 
@@ -226 +226 @@
     public int runBasicProcess() {
     try {
+        this.forciblyTerminateProcess = true;
+        
         // 1. create the process
         process = doRuntimeExec();
@@ -231 +233 @@
         this.exitValue = process.waitFor();
 
-        
-    } catch(IOException ioe) {
+        this.forciblyTerminateProcess = false;
+    } catch(IOException ioe) {      
+        
         if(exceptionHandler != null) {
         exceptionHandler.gotException(ioe);
@@ -239 +242 @@
         }
     } catch(InterruptedException ie) {
-
+        
         if(exceptionHandler != null) {
         exceptionHandler.gotException(ie);
@@ -249 +252 @@
     } finally { 
 
-        if( process != null ) {
-        process.destroy(); // see runProcess() below
-        }
+        if( this.forciblyTerminateProcess ) {
+        process.destroy(); // see runProcess() below        
+        }
+        process = null;
+        this.forciblyTerminateProcess = false; // reset
     }
     return this.exitValue;
@@ -273 +278 @@
 
     try {
+        this.forciblyTerminateProcess = false;
+        
         // 1. get the Process object
         process = doRuntimeExec();
@@ -311 +318 @@
        
     } catch(IOException ioe) {
+        this.forciblyTerminateProcess = true;
+
         if(exceptionHandler != null) {
         exceptionHandler.gotException(ioe);
@@ -317 +326 @@
         }
     } catch(InterruptedException ie) { // caused during any of the gobblers.join() calls, this is unexpected so print stack trace
-        
+        this.forciblyTerminateProcess = true;
+        
         if(exceptionHandler != null) {
         exceptionHandler.gotException(ie);
@@ -332 +342 @@
         //log("*** In finally of SafeProcess.runProcess(3 params): " + cmd);
 
-        if( process != null ) {
+        if( this.forciblyTerminateProcess ) {
         log("*** Going to call process.destroy 2");
         process.destroy();
-        process = null;
         log("*** Have called process.destroy 2");
         }
-        
+        process = null;
+        this.forciblyTerminateProcess = false; // reset
     }
     
@@ -351 +361 @@
 
     try {
+        this.forciblyTerminateProcess = false;
+        
         // 1. get the Process object
         process = doRuntimeExec();
@@ -383 +395 @@
        
     } catch(IOException ioe) {
+        this.forciblyTerminateProcess = true;
+        
         if(exceptionHandler != null) {
         exceptionHandler.gotException(ioe);
@@ -389 +403 @@
         }
     } catch(InterruptedException ie) { // caused during any of the gobblers.join() calls, this is unexpected so log it
-        
+        this.forciblyTerminateProcess = true;
+        
         if(exceptionHandler != null) {
         exceptionHandler.gotException(ie);
@@ -415 +430 @@
         //log("*** In finally of SafeProcess.runProcess(2 params): " + cmd);
 
-        if( process != null ) {
+        if( this.forciblyTerminateProcess ) {
         log("*** Going to call process.destroy 1");
         process.destroy();
+        log("*** Have called process.destroy 1");
+        }
         process = null;
-        log("*** Have called process.destroy 1");
-        }
+        this.forciblyTerminateProcess = false; //reset      
     }