Changeset 31702 for main/trunk/gli/src

Timestamp:

2017-05-25T18:09:55+12:00 (7 years ago)

Author:

ak19

Message:

1. Correction to previous commit where I'd accidentally attempted a forced KILL request first instead of sending the preferred TERMination request first. The sequence should be the inverse. 2. Adding the command to kill a process tree on linux. We don't seem to need it at present, since an interruption (cancel action) cleanly stops a process and its subprocesses on linux without even requiring a process.destroy() call. But adding the code into SafeProcess.java for reusability.

File:

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

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

@@ -731 +731 @@
     
 
-// stackoverflow.com/questions/1835885/send-ctrl-c-to-process-open-by-java  
+// Can't artificially send Ctrl-C: stackoverflow.com/questions/1835885/send-ctrl-c-to-process-open-by-java
 // (Taskkill command can kill all running perl. But what if we kill perl instances not launched by GS?)
 // stackoverflow.com/questions/912889/how-to-send-interrupt-key-sequence-to-a-java-process
@@ -756 +756 @@
 // but the other suggestion did work: pkill -TERM -P pid did work
 // More reading:
+// https://linux.die.net/man/1/kill (manual)
 // https://unix.stackexchange.com/questions/117227/why-pidof-and-pgrep-are-behaving-differently
 // https://unix.stackexchange.com/questions/67635/elegantly-get-list-of-children-processes
 // https://stackoverflow.com/questions/994033/mac-os-x-quickest-way-to-kill-quit-an-entire-process-tree-from-within-a-cocoa-a
-// https://unix.stackexchange.com/questions/132224/is-it-possible-to-get-process-group-id-from-procŧŧ
+// https://unix.stackexchange.com/questions/132224/is-it-possible-to-get-process-group-id-from-proc
+// https://unix.stackexchange.com/questions/99112/default-exit-code-when-process-is-terminated
 
 /**
@@ -766 +768 @@
  * if false, will send the -TERM (-15) signal, which will allow cleanup before termination. Sending a SIGTERM is preferred.
  * @param killEntireTree if false, will terminate only the process denoted by processID, otherwise all descendants/subprocesses too.
- * @return true if running the kill process returned an exit value of 0 
+ * @return true if running the kill process returned an exit value of 0 or if it had already been terminated
 */
 static boolean killUnixProcessWithID(long processID, boolean force, boolean killEntireTree) {
@@ -773 +775 @@
     String cmd;
     if(killEntireTree) { // kill the process denoted by processID and any subprocesses this launched
-    cmd = "pkill -"+signal + " -P " + processID; // e.g. "pkill -TERM -P pid"
+
+    if(Utility.isMac()) { 
+        // this cmd works on Mac (tested Snow Leopard), but on Linux this cmd only terminates toplevel process
+        // when doing full-import, and doesn't always terminate subprocesses when doing full-buildcol.pl
+        cmd = "pkill -"+signal + " -P " + processID; // e.g. "pkill -TERM -P pid"
+    } 
+    else { // other unix
+        // this cmd works on linux, not recognised on Mac (tested Snow Leopard):
+        cmd = "kill -"+signal + " -"+processID; // e.g. "kill -TERM -pid"
+                                                // note the hyphen before pid to terminate subprocesses too
+    }
+
     } else { // kill only the process represented by the processID.
     cmd = "kill -"+signal + " " + processID; // e.g. "kill -TERM pid"
@@ -780 +793 @@
     SafeProcess proc = new SafeProcess(cmd);
     int exitValue = proc.runProcess();
-    if(exitValue != 0) {
-    log("@@@ Not able to successfull terminate process, got exitvalue " + exitValue);
-    log("@@@ Got output " + proc.getStdOutput()); 
-    log("@@@ Got err output " + proc.getStdError());
-    // caller can try again with kill -KILL, by setting force parameter to true
-    return false;
-    } else {
+    
+    
+    if(exitValue == 0) {
     if(force) {
         log("@@@ Successfully sent SIGKILL to unix process tree rooted at " + processID);
@@ -793 +802 @@
     }
     return true;
-    }    
+    } else if(!Utility.isMac() && exitValue == 1) {
+    // https://stackoverflow.com/questions/28332888/return-value-of-kill
+    // "kill returns an exit code of 0 (true) if the process still existed it and was killed.
+    // kill returns an exit code of 1 (false) if the kill failed, probably because the process was no longer running."
+    // On Linux, interrupting the process and its worker threads and closing resources already successfully terminates
+    // the process and its subprocesses (don't need to call this method at all to terminate the processes: the processes
+    // aren't running when we get to this method)
+    log("@@@ Termination signal returned exitvalue 1, on linux this happens when the process has already been terminated");
+    return true;
+    } else {
+    log("@@@ Not able to successfully terminate process, got exitvalue " + exitValue);
+    log("@@@ Got output: |" + proc.getStdOutput() + "|"); 
+    log("@@@ Got err output: |" + proc.getStdError() + "|");
+    // caller can try again with kill -KILL, by setting force parameter to true
+    return false;
+    }
 }
 
@@ -803 +827 @@
     log("### in SafeProcess.destroyProcess(Process p)");
 
-    String osName = Utility.getOSdirName();
-
     // If it isn't windows, process.destroy() terminates any child processes too
-    if(osName.equals("linux")) {
-    p.destroy();
-    return;
-    }
-
-    if(osName.equals("darwin")) {
+    if(!Utility.isWindows()) {
     long pid = SafeProcess.getProcessID(p);
     /*
@@ -821 +838 @@
         p.destroy(); // at minimum, will have no effect if the process had already terminated 
     } else {
-        boolean forceKill = false;
+        boolean forceKill = true;
         boolean killEntireProcessTree = true;
+        
         if(!killUnixProcessWithID(pid, !forceKill, killEntireProcessTree)) { // send sig TERM (kill -15 or kill -TERM)
         killUnixProcessWithID(pid, forceKill, killEntireProcessTree); // send sig KILL (kill -9 or kill -KILL)
-        }
+        }       
     }