Changeset 31638

Show
Ignore:
Timestamp:
01.05.2017 18:23:39 (2 years ago)
Author:
ak19
Message:

Rewriting GShell.runLocal() to use SafeProcess? instead of Process. This required overhaul of SafeProcess? to correctly deal with interrupts as the build/import process launched in GShell can be cancelled from the CreatePane?. SafeProcess? now also keeps track of its internal process object and has static logging methods to simplify the changes required when swapping between the GS3 version and GS2 version of SafeProcess?. May eventually call DebugStream? methods from the log methods for GLI. Will add a DEBUG flag to allow verbose logging.

Location:
main/trunk/gli/src/org/greenstone/gatherer
Files:
4 modified

Legend:

Unmodified
Added
Removed
  • main/trunk/gli/src/org/greenstone/gatherer/collection/CollectionManager.java

    r31190 r31638  
    246246    GShell shell = new GShell(command_parts, GShell.BUILD, BUILDING, this, build_monitor, GShell.GSHELL_BUILD); 
    247247    shell.addGShellListener(Gatherer.g_man.create_pane); 
     248    Gatherer.g_man.create_pane.setGShell(shell); // allow the create pane to call shell.cancel() 
    248249    shell.addGShellListener(Gatherer.g_man.format_pane); 
    249250    shell.start(); 
     
    367368        GShell shell = new GShell(sched_parts, GShell.SCHEDULE, SCHEDULING, this, schedule_monitor, GShell.GSHELL_SCHEDULE); 
    368369    shell.addGShellListener(Gatherer.g_man.create_pane); 
     370    Gatherer.g_man.create_pane.setGShell(shell); // allow the create pane to call shell.cancel() 
    369371    shell.addGShellListener(Gatherer.g_man.format_pane); 
    370372    shell.start(); 
     
    12271229    //shell.setEventProperty("is_incremental", Boolean.toString(is_incremental)); 
    12281230    shell.addGShellListener(Gatherer.g_man.create_pane); 
     1231    Gatherer.g_man.create_pane.setGShell(shell); // allow the create pane to call shell.cancel() 
    12291232        shell.addGShellListener(Gatherer.g_man.format_pane); 
    12301233    shell.start(); 
  • main/trunk/gli/src/org/greenstone/gatherer/gui/CreatePane.java

    r29711 r31638  
    156156    public String homepage = null; 
    157157 
     158    /** Access to the GShell object that this CreatePane listens to events for.  
     159     * A handle to the GShell is needed order to interrupt any processes the GShell runs 
     160     * when the user cancels a build operation. 
     161    */ 
     162    private GShell shell = null; 
     163 
    158164    /** The constructor creates important helper classes, and initializes all the components. 
    159165     * @see org.greenstone.gatherer.collection.CollectionManager 
     
    291297    } 
    292298 
     299    public void setGShell(GShell shell) { 
     300    this.shell = shell; 
     301    } 
    293302 
    294303    public void destroy() { 
     
    840849    private class CancelButtonListener 
    841850    implements ActionListener { 
    842     /** This method attempts to cancel the current GShell task. It does this by first telling CollectionManager not to carry out any further action. This it turn tells the GShell to break from the current job immediately, without waiting for the processEnded message, and then kills the thread in an attempt to stop the process. The results of such an action are debatable. 
     851    /** This method attempts to cancel the current GShell task. It does this by first telling CollectionManager not to carry out any further action.  
     852     * Previously, this would in turn tell the GShell to break from the current job immediately, without waiting for the processEnded message, and then kills the thread in an attempt to stop the process. The results of such an action are debatable. 
     853     * Now, pressing cancel will send an interrupt to the GShell thread, which is the thread in which the external process is run (via the SafeProcess object). Interrupting a running SafeProcess will then interrupt any worker threads and destroy the process, with SafeProcess cleaning up after itself after its worker threads finished cleaning up after themselves. Tested on Linux. 
    843854     * @param event An <strong>ActionEvent</strong> who, thanks to the power of object oriented programming, we don't give two hoots about. 
    844855     * @see org.greenstone.gatherer.Gatherer 
     
    867878        build_monitor.setStop(true); 
    868879        build_monitor.reset(); 
     880        // Tell the GShell to cleanly stop running its external process 
     881        if(CreatePane.this.shell != null) { 
     882        // We can call GShell.cancel() even if the GShell thread is blocking when running a process, 
     883        // because this CreatePane is running in its own separate GUI thread. This is because the 
     884        // process blocking call (process.waitFor()) and cancel() are not sequential operations. 
     885        CreatePane.this.shell.cancel(); 
     886        } 
    869887 
    870888        // Remove the collection lock. 
  • main/trunk/gli/src/org/greenstone/gatherer/shell/GShell.java

    r26574 r31638  
    5656import org.greenstone.gatherer.metadata.DocXMLFileManager; 
    5757import org.greenstone.gatherer.remote.RemoteGreenstoneServer; 
    58 import org.greenstone.gatherer.util.StaticStrings;  
     58import org.greenstone.gatherer.util.SafeProcess; 
     59import org.greenstone.gatherer.util.StaticStrings; 
    5960import org.greenstone.gatherer.util.Utility; 
    6061 
     
    6364 */ 
    6465public class GShell 
    65     extends Thread { 
     66    extends Thread implements SafeProcess.ExceptionHandler { 
    6667    /** A flag used to determine if this process has been asked to cancel. */ 
    6768    private boolean cancel = false; 
     
    8485    private String commandOutput = null; 
    8586 
     87    /** The process safely run in this GShell. Make sure to set to null when done with. */ 
     88    SafeProcess prcs = null; 
     89 
    8690    /** Elements in process type enumeration. */ 
    8791    static final public int BUILD = 0; 
     
    113117    static public String GSHELL_FEDORA_COLDELETE = "gshell_fedora_col_delete"; 
    114118 
    115     /** Determine if the given process is still executing. It does this by attempting to throw an exception - not the most efficient way, but the only one as far as I know 
    116      * @param process the Process to test 
    117      * @return true if it is still executing, false otherwise 
    118      */ 
    119     static public boolean processRunning(Process process) { 
    120     boolean process_running = false; 
    121  
    122     try { 
    123         process.exitValue(); // This will throw an exception if the process hasn't ended yet. 
    124     } 
    125     catch(IllegalThreadStateException itse) { 
    126         process_running = true; 
    127     } 
    128     catch(Exception exception) { 
    129         DebugStream.printStackTrace(exception); 
    130     } 
    131     return process_running; 
    132     } 
    133119 
    134120    /** Constructor gatherer all the data required to create a new process, and emit meaningfull messages. 
     
    236222 
    237223 
    238     private void runLocal(String[] args, BufferedOutputStream bos)  
     224    // old_runLocal uses a potentially unsafe way of running a process and an inefficient way of reading 
     225    // from the process stdout and stderr streams. Replaced by new runLocal() which uses SafeProcess and 
     226    // the new CustomProcessHandler inner class instantiated for each of the 2 process output streams. 
     227    private void old_runLocal(String[] args, BufferedOutputStream bos)  
    239228    { 
    240229    try { 
     
    260249        StringBuffer stdline_buffer = new StringBuffer(); 
    261250     
    262         while(/*type != GShell.NEW &&*/ processRunning(prcs) && !hasSignalledStop()) { 
     251        while(/*type != GShell.NEW &&*/ SafeProcess.processRunning(prcs) && !hasSignalledStop()) { 
    263252        // Hopefully this doesn't block if the process is trying to write to STDOUT. 
    264253        if((eisr!=null) && eisr.ready()) {  
     
    348337    } 
    349338     
     339    private void runLocal(String[] args, BufferedOutputStream bos)  
     340    { 
     341    // in case we stop between import and build, let's not launch further processes 
     342    // that only pressing cancel, which results in interrupts, can subsequently stop. 
     343    if(hasSignalledStop()) { 
     344        return; 
     345    } 
     346 
     347    String command = ""; 
     348    for(int i = 0; i < args.length; i++) { 
     349        command = command + args[i] + " "; 
     350    } 
     351     
     352    ///ystem.err.println("Command: " + command); 
     353    fireMessage(type, Dictionary.get("GShell.Command") + ": " + command, status, null); 
     354     
     355    prcs = new SafeProcess(args); 
     356    SafeProcess.CustomProcessHandler processOutHandler 
     357        = new SynchronizedProcessHandler(bos, SafeProcess.STDOUT); 
     358    SafeProcess.CustomProcessHandler processErrHandler 
     359        = new SynchronizedProcessHandler(bos, SafeProcess.STDERR); 
     360     
     361    prcs.setExceptionHandler(this);  
     362    int exitValue = prcs.runProcess(null, processOutHandler, processErrHandler); // use default procIn handling 
     363     
     364    if(exitValue == 0) { 
     365        System.err.println("*** Exited normally"); 
     366        status = OK; 
     367        fireMessage(type, typeAsString(type) + "> " + Dictionary.get("GShell.Success"), status, null); 
     368    } 
     369    else { 
     370        System.err.println("*** Exited abnormally with exit value " + exitValue); 
     371        status = ERROR; 
     372        fireMessage(type, typeAsString(type) + "> " + Dictionary.get("GShell.Failure"), status, null); 
     373    } 
     374     
     375    prcs = null; 
     376    } 
     377 
     378    /** When cancel is called on this GShell thread from a separate thread,  
     379     * This GShell will safely terminate any process it's currently running (by interrupting it) 
     380     * and will set this.prcs to null at the end. */ 
     381    public void cancel() { 
     382    if(prcs != null) { //GShell is running a process, so interrupt the GShell/SafeProcess thread  
     383 
     384        SafeProcess.log("********** HAS SIGNALLED STOP. INTERRUPTING THE GSHELL/SAFEPROCESS THREAD"); 
     385 
     386        this.interrupt(); // interrupt this thread which is running the SafeProcess prcs 
     387        // this will propagate the CANCEL status 
     388        prcs = null;         
     389    } 
     390    hasSignalledStop(); // synchronized. Will set status to CANCELLED in thread-safe manner.     
     391    } 
    350392     
    351393 
     
    589631     * @return A <strong>boolean</strong> indicating if the user wanted to stop. 
    590632     */ 
    591     public boolean hasSignalledStop() { 
     633    public synchronized boolean hasSignalledStop() { 
    592634    boolean has_signalled_stop = false; 
    593635    if(progress != null) { 
     
    639681    return name; 
    640682    } 
     683 
     684    // From interface SafeProcess.ExceptionHandler 
     685    // Called when an exception happens during the running of our perl process, as we want to set 
     686    // the GShell status to ERROR. 
     687    // However, exceptions when reading from our perl process' stderr and stdout streams are handled 
     688    // by SynchronizedProcessHandler.gotException() below, since they happen in separate threads 
     689    // from this one (the one from which the perl process is run). 
     690    public void gotException(Exception e) {  
     691 
     692    if(e instanceof InterruptedException) { 
     693        SafeProcess.log("*** InterruptedException in GShell.runLocal()"); 
     694        setStatus(CANCELLED); // expected exception 
     695    } else { 
     696        DebugStream.println("Exception in GShell.runLocal() - unexpected"); 
     697        DebugStream.printStackTrace(e); 
     698        setStatus(ERROR); // status particularly needs to be synchronized on 
     699    } 
     700    } 
     701 
     702    public synchronized void setStatus(int newStatus) { 
     703    status = newStatus; 
     704    } 
     705 
     706    // Each instance of this class is run in its own thread by class SafeProcess.InputGobbler. 
     707    // This class deals with each incoming line from the perl process' stderr or stdout streams. One 
     708    // instance of this class for each stream. However, since multiple instances of this CustomProcessHandler 
     709    // could be (and in fact, are) writing to the same file in their own threads, several objects, not just 
     710    // the bufferedwriter object, needed to be made threadsafe. 
     711    // This class also handles exceptions during the running of the perl process. 
     712    // The runPerlCommand code originally would do a sendProcessStatus on each exception, so we ensure 
     713    // we do that here too, to continue original behaviour. These calls are also synchronized to make their 
     714    // use of the EventListeners threadsafe. 
     715    protected class SynchronizedProcessHandler extends SafeProcess.CustomProcessHandler 
     716    { 
     717    private final BufferedOutputStream bos; // needs to be final to be able to synchronize on the shared object 
     718     
     719    public SynchronizedProcessHandler(BufferedOutputStream bos, int src) { 
     720        super(src); // will set this.source to STDERR or STDOUT 
     721        this.bos = bos; // caller will close bw, since many more than one 
     722                            // SynchronizedProcessHandlers are using it 
     723    } 
     724 
     725    // trying to keep synchronized methods as short as possible 
     726    private void logException(Exception e) { 
     727        String stream = (this.source == SafeProcess.STDERR) ? "stderr" : "stdout"; 
     728        String msg = "Got exception when processing the perl process' " + stream + " stream."; 
     729 
     730        DebugStream.println(msg); // method is already synchronized 
     731        DebugStream.printStackTrace(e); // method is already synchronized 
     732        SafeProcess.log("*** " + msg, e); 
     733 
     734        GShell.this.setStatus(GShell.ERROR); 
     735    } 
     736 
     737    private void log(String msg) { 
     738        DebugStream.println(msg); // already synchro 
     739        System.err.println("@@@@@ " + msg); 
     740    } 
     741 
     742    public void run(Closeable inputStream) { 
     743        InputStream is = (InputStream) inputStream; 
     744 
     745        BufferedReader br = null; 
     746        try {        
     747 
     748        br = new BufferedReader(new InputStreamReader(is, "UTF-8")); 
     749        String line=null; 
     750 
     751        ///while ( !GShell.this.prcs.processRunning() || hasSignalledStop()) { // need to sync on prcs 
     752        while ( !Thread.currentThread().isInterrupted() && (line = br.readLine()) != null ) { 
     753            this.gotLine(line); // synchronized          
     754        }        
     755 
     756        } catch (Exception e) { 
     757        logException(e); 
     758        } finally { 
     759        System.err.println("*********"); 
     760        if(Thread.currentThread().isInterrupted()) { 
     761            log("We've been asked to stop."); 
     762             
     763        } 
     764        SafeProcess.closeResource(br); 
     765        String stream = (this.source == SafeProcess.STDERR) ? "stderr" : "stdout"; 
     766        System.err.println("*** In finally of " + stream + " stream"); 
     767        System.err.println("*********"); 
     768        } 
     769    } 
     770 
     771    protected synchronized void gotLine(String line) { 
     772        fireMessage(GShell.this.type,   // not final, needs synchro 
     773            GShell.this.typeAsString(type) + "> " + line, 
     774            GShell.this.status, // not final, needs synchro 
     775            this.bos);          // needs synchro 
     776    } 
     777 
     778    } 
     779 
    641780} 
  • main/trunk/gli/src/org/greenstone/gatherer/util/SafeProcess.java

    r31630 r31638  
    1010import java.io.OutputStream; 
    1111import java.io.OutputStreamWriter; 
    12  
    1312import java.util.Arrays; 
    1413 
    1514import org.apache.log4j.*; 
     15 
     16import org.greenstone.gatherer.DebugStream; 
    1617 
    1718// Use this class to run a Java Process. It follows the good and safe practices at 
     
    2324    public static final int STDERR = 0; 
    2425    public static final int STDOUT = 1; 
    25     public static final int STDIN = 2;     
     26    public static final int STDIN = 2; 
     27 
     28    // charset for reading process stderr and stdout streams 
     29    //public static final String UTF8 = "UTF-8";     
    2630 
    2731    ///static Logger logger = Logger.getLogger(org.greenstone.util.SafeProcess.class.getName()); 
     
    3337    private File dir = null; 
    3438    private String inputStr = null; 
     39    private Process process = null; 
    3540 
    3641    // output from running SafeProcess.runProcess() 
     
    3843    private String errorStr = ""; 
    3944    private int exitValue = -1; 
     45    //private String charset = null; 
    4046 
    4147    // allow callers to process exceptions of the main process thread if they want 
     
    7480    public int getExitValue() { return exitValue; } 
    7581 
    76      
     82    //public void setStreamCharSet(String charset) { this.charset = charset; }  
     83 
    7784    // set any string to send as input to the process spawned by SafeProcess 
    7885    public void setInputString(String sendStr) { 
     
    94101    } 
    95102 
     103    // logger and DebugStream print commands are synchronized, therefore thread safe. 
     104    public static void log(String msg) { 
     105    //logger.info(msg); 
     106 
     107    System.err.println(msg); 
     108 
     109    //DebugStream.println(msg); 
     110    } 
     111 
     112    public static void log(String msg, Exception e) { // Print stack trace on the exception 
     113    //logger.error(msg, e); 
     114 
     115    System.err.println(msg); 
     116    e.printStackTrace(); 
     117 
     118    //DebugStream.println(msg); 
     119    //DebugStream.printStackTrace(e); 
     120    } 
     121 
     122    public static void log(Exception e) { 
     123    //logger.error(e); 
     124 
     125    e.printStackTrace(); 
     126 
     127    //DebugStream.printStackTrace(e); 
     128    } 
     129 
     130    public static void log(String msg, Exception e, boolean printStackTrace) { 
     131    if(printStackTrace) {  
     132        log(msg, e); 
     133    } else { 
     134        log(msg); 
     135    } 
     136    } 
     137 
    96138    private Process doRuntimeExec() throws IOException { 
    97139    Process prcs = null; 
     
    99141     
    100142    if(this.command != null) { 
    101         //logger.info("SafeProcess running: " + command); 
    102         System.err.println("SafeProcess running: " + command_args); 
     143        log("SafeProcess running: " + command); 
    103144        prcs = rt.exec(this.command); 
    104145    } 
     
    106147         
    107148        // http://stackoverflow.com/questions/5283444/convert-array-of-strings-into-a-string-in-java 
    108         //logger.info("SafeProcess running: " + Arrays.toString(command_args)); 
    109         System.err.println("SafeProcess running: " + Arrays.toString(command_args)); 
     149        log("SafeProcess running: " + Arrays.toString(command_args)); 
    110150         
    111151        if(this.envp == null) {  
     
    114154         
    115155        if(this.dir == null) { 
    116             ///logger.info("\twith: " + Arrays.toString(this.envp)); 
    117             ///System.err.println("\twith: " + Arrays.toString(this.envp)); 
     156            //log("\twith: " + Arrays.toString(this.envp)); 
    118157            prcs = rt.exec(this.command_args, this.envp); 
    119158        } else { 
    120             ///logger.info("\tfrom directory: " + this.dir); 
    121             ///logger.info("\twith: " + Arrays.toString(this.envp)); 
    122             ///System.err.println("\tfrom directory: " + this.dir); 
    123             ///System.err.println("\twith: " + Arrays.toString(this.envp)); 
    124             prcs = rt.exec(this.command_args, this.envp, this.dir); 
     159            //log("\tfrom directory: " + this.dir); 
     160            //log("\twith: " + Arrays.toString(this.envp));          
     161            prcs = rt.exec(this.command_args, this.envp, this.dir); 
    125162        } 
    126163        } 
     
    131168 
    132169    // Copied from gli's gui/FormatConversionDialog.java 
    133     private int waitForWithStreams(Process prcs, 
    134                    SafeProcess.OutputStreamGobbler inputGobbler, 
     170    private int waitForWithStreams(SafeProcess.OutputStreamGobbler inputGobbler, 
    135171                   SafeProcess.InputStreamGobbler outputGobbler, 
    136172                   SafeProcess.InputStreamGobbler errorGobbler) 
     
    138174    { 
    139175 
    140             // kick off the stream gobblers 
    141             inputGobbler.start(); 
    142             errorGobbler.start(); 
    143             outputGobbler.start(); 
    144  
    145             // any error??? 
    146             this.exitValue = prcs.waitFor(); // can throw an InterruptedException if process did not terminate 
    147  
    148             ///logger.info("Process exitValue: " + exitValue); 
    149         ///System.err.println("Process exitValue: " + exitValue);  
    150  
     176     
     177    // kick off the stream gobblers 
     178    inputGobbler.start(); 
     179    errorGobbler.start(); 
     180    outputGobbler.start(); 
     181     
     182    // any error??? 
     183    try { 
     184        this.exitValue = process.waitFor(); // can throw an InterruptedException if process did not terminate 
     185    } catch(InterruptedException ie) { 
     186         
     187        log("*** Process interrupted (InterruptedException). Expected to be a Cancel operation."); 
     188            // don't print stacktrace: an interrupt here is not an error, it's expected to be a cancel action 
     189         
     190        // propagate interrupts to worker threads here 
     191        // unless the interrupt emanated from any of them in any join(), 
     192        // which will be caught by caller's catch on InterruptedException. 
     193        // Only if the thread that SafeProcess runs in was interrupted 
     194        // should we propagate the interrupt to the worker threads. 
     195        // http://stackoverflow.com/questions/2126997/who-is-calling-the-java-thread-interrupt-method-if-im-not 
     196        // "I know that in JCiP it is mentioned that you should never interrupt threads you do not own" 
     197        // But SafeProcess owns the worker threads, so it has every right to interrupt them 
     198        // Also read http://stackoverflow.com/questions/13623445/future-cancel-method-is-not-working?noredirect=1&lq=1 
     199         
     200        // http://stackoverflow.com/questions/3976344/handling-interruptedexception-in-java 
     201        // http://stackoverflow.com/questions/4906799/why-invoke-thread-currentthread-interrupt-when-catch-any-interruptexception 
     202        // "Only code that implements a thread's interruption policy may swallow an interruption request. General-purpose task and library code should never swallow interruption requests." 
     203        // Does that mean that since this code implements this thread's interruption policy, it's ok 
     204        // to swallow the interrupt this time and not let it propagate by commenting out the next line? 
     205        //Thread.currentThread().interrupt(); // re-interrupt the thread 
     206         
     207        inputGobbler.interrupt(); 
     208        errorGobbler.interrupt(); 
     209        outputGobbler.interrupt(); 
     210         
     211    } finally {      
     212         
     213        //log("Process exitValue: " + exitValue); 
     214         
    151215        // From the comments of  
    152216        // http://www.javaworld.com/article/2071275/core-java/when-runtime-exec---won-t.html?page=2 
     
    154218        // if there's no waiting for the threads, call join() on each Thread (StreamGobbler) object. 
    155219        // From Thread API: join() "Waits for this thread (the thread join() is invoked on) to die." 
     220         
     221        // Wait for each of the threads to die, before attempting to destroy the process 
     222        // Any of these can throw InterruptedExceptions too 
     223        // and will be processed by the calling function's catch on InterruptedException. 
     224        // However, no one besides us will interrupting these threads I think... 
     225        // and we won't be throwing the InterruptedException from within the threads... 
     226        // So if any streamgobbler.join() call throws an InterruptedException, that would be unexpected 
     227 
    156228        outputGobbler.join();  
    157229        errorGobbler.join(); 
    158         inputGobbler.join();  
    159          
    160  
    161         // set the variables the code that created a SafeProcess object may want to inspect 
     230        inputGobbler.join(); 
     231         
     232         
     233        // set the variables that the code which created a SafeProcess object may want to inspect 
    162234        this.outputStr = outputGobbler.getOutput(); 
    163235        this.errorStr = errorGobbler.getOutput(); 
     
    165237        // Since we didn't have an exception, process should have terminated now (waitFor blocks until then) 
    166238        // Set process to null so we don't forcibly terminate it below with process.destroy() 
    167         prcs = null; 
    168  
    169         return this.exitValue; 
    170     } 
    171  
     239        this.process = null;         
     240    } 
     241 
     242    // Don't return from finally, it's considered an abrupt completion and exceptions are lost, see 
     243    // http://stackoverflow.com/questions/18205493/can-we-use-return-in-finally-block 
     244    return this.exitValue; 
     245    } 
     246 
     247 
     248    public synchronized boolean processRunning() { 
     249    if(process == null) return false; 
     250    return SafeProcess.processRunning(this.process); 
     251    } 
    172252 
    173253    // Run a very basic process: with no reading from or writing to the Process' iostreams, 
    174254    // this just execs the process and waits for it to return 
    175255    public int runBasicProcess() { 
    176     Process prcs = null; 
    177256    try { 
    178257        // 1. create the process 
    179         prcs = doRuntimeExec(); 
     258        process = doRuntimeExec(); 
    180259        // 2. basic waitFor the process to finish 
    181         this.exitValue = prcs.waitFor(); 
     260        this.exitValue = process.waitFor(); 
    182261 
    183262         
     
    186265        exceptionHandler.gotException(ioe); 
    187266        } else { 
    188         //logger.error("IOException: " + ioe.getMessage(), ioe); 
    189         System.err.println("IOException " + ioe.getMessage()); 
    190         ioe.printStackTrace(); 
     267        log("IOException: " + ioe.getMessage(), ioe); 
    191268        } 
    192269    } catch(InterruptedException ie) { 
     
    194271        if(exceptionHandler != null) { 
    195272        exceptionHandler.gotException(ie); 
    196         } else { 
    197         //logger.error("Process InterruptedException: " + ie.getMessage(), ie); 
    198         System.err.println("Process InterruptedException " + ie.getMessage()); 
    199         ///ie.printStackTrace(); // an interrupt here is not an error, it can be a cancel action 
     273        } else { // Unexpected InterruptedException, so printstacktrace 
     274        log("Process InterruptedException: " + ie.getMessage(), ie); 
    200275        } 
    201276         
     
    203278    } finally {  
    204279 
    205         if( prcs != null ) { 
    206         prcs.destroy(); // see runProcess() below 
     280        if( process != null ) { 
     281        process.destroy(); // see runProcess() below 
    207282        } 
    208283    } 
     
    222297               CustomProcessHandler procErrHandler) 
    223298    { 
    224     Process prcs = null; 
    225299    SafeProcess.OutputStreamGobbler inputGobbler = null; 
    226300    SafeProcess.InputStreamGobbler errorGobbler = null; 
     
    229303    try { 
    230304        // 1. get the Process object 
    231         prcs = doRuntimeExec(); 
     305        process = doRuntimeExec(); 
    232306         
    233307 
     
    238312        // send inputStr to process. The following constructor can handle inputStr being null 
    239313        inputGobbler = // WriterToProcessInputStream 
    240             new SafeProcess.OutputStreamGobbler(prcs.getOutputStream(), this.inputStr); 
     314            new SafeProcess.OutputStreamGobbler(process.getOutputStream(), this.inputStr); 
    241315        } else { // user will do custom handling of process' InputStream  
    242         inputGobbler = new SafeProcess.OutputStreamGobbler(prcs.getOutputStream(), procInHandler); 
     316        inputGobbler = new SafeProcess.OutputStreamGobbler(process.getOutputStream(), procInHandler); 
    243317        } 
    244318 
     
    246320        if(procErrHandler == null) { 
    247321        errorGobbler // ReaderFromProcessOutputStream 
    248             = new SafeProcess.InputStreamGobbler(prcs.getErrorStream(), splitStdErrorNewLines); 
     322            = new SafeProcess.InputStreamGobbler(process.getErrorStream(), splitStdErrorNewLines); 
    249323        } else { 
    250324        errorGobbler 
    251             = new SafeProcess.InputStreamGobbler(prcs.getErrorStream(), procErrHandler); 
     325            = new SafeProcess.InputStreamGobbler(process.getErrorStream(), procErrHandler); 
    252326        } 
    253327 
     
    255329        if(procOutHandler == null) { 
    256330        outputGobbler 
    257             = new SafeProcess.InputStreamGobbler(prcs.getInputStream(), splitStdOutputNewLines); 
     331            = new SafeProcess.InputStreamGobbler(process.getInputStream(), splitStdOutputNewLines); 
    258332        } else { 
    259333        outputGobbler 
    260             = new SafeProcess.InputStreamGobbler(prcs.getInputStream(), procOutHandler); 
     334            = new SafeProcess.InputStreamGobbler(process.getInputStream(), procOutHandler); 
    261335        } 
    262336 
    263337 
    264338            // 3. kick off the stream gobblers 
    265         this.exitValue = waitForWithStreams(prcs, inputGobbler, outputGobbler, errorGobbler); 
     339        this.exitValue = waitForWithStreams(inputGobbler, outputGobbler, errorGobbler); 
    266340        
    267341    } catch(IOException ioe) { 
     
    269343        exceptionHandler.gotException(ioe); 
    270344        } else { 
    271         //logger.error("IOexception: " + ioe.getMessage(), ioe); 
    272         System.err.println("IOexception " + ioe.getMessage()); 
    273         ioe.printStackTrace(); 
    274         } 
    275     } catch(InterruptedException ie) { 
    276  
     345        log("IOexception: " + ioe.getMessage(), ioe); 
     346        } 
     347    } catch(InterruptedException ie) { // caused during any of the gobblers.join() calls, this is unexpected so print stack trace 
     348         
    277349        if(exceptionHandler != null) { 
    278350        exceptionHandler.gotException(ie); 
     351        log("@@@@ Unexpected InterruptedException when waiting for process stream gobblers to die"); 
    279352        } else { 
    280         //logger.error("Process InterruptedException: " + ie.getMessage(), ie); 
    281         System.err.println("Process InterruptedException " + ie.getMessage()); 
    282         ///ie.printStackTrace(); // an interrupt here is not an error, it can be a cancel action 
    283         } 
    284  
    285         // propagate interrupts to worker threads here? 
    286         // unless the interrupt emanated from any of them in any join()... 
    287         // Only if the thread SafeProcess runs in was interrupted 
    288         // do we propagate the interrupt to the worker threads. 
    289         // http://stackoverflow.com/questions/2126997/who-is-calling-the-java-thread-interrupt-method-if-im-not 
    290         // "I know that in JCiP it is mentioned that you should never interrupt threads you do not own" 
    291         // But SafeProcess owns the worker threads, so it have every right to interrupt them 
    292         // Also read http://stackoverflow.com/questions/13623445/future-cancel-method-is-not-working?noredirect=1&lq=1 
    293         if(Thread.currentThread().isInterrupted()) { 
    294         inputGobbler.interrupt(); 
    295         errorGobbler.interrupt(); 
    296         outputGobbler.interrupt();       
    297         } 
    298  
    299         // On catchingInterruptedException, re-interrupt the thread. 
    300         // This is just how InterruptedExceptions tend to be handled 
    301         // See also http://stackoverflow.com/questions/4906799/why-invoke-thread-currentthread-interrupt-when-catch-any-interruptexception 
    302         // and https://praveer09.github.io/technology/2015/12/06/understanding-thread-interruption-in-java/ 
    303  
    304         // http://stackoverflow.com/questions/3976344/handling-interruptedexception-in-java 
    305         // http://stackoverflow.com/questions/4906799/why-invoke-thread-currentthread-interrupt-when-catch-any-interruptexception 
    306         // "Only code that implements a thread's interruption policy may swallow an interruption request. General-purpose task and library code should never swallow interruption requests." 
    307         // Does that mean that since this code implements this thread's interruption policy, it's ok 
    308         // to swallow the interrupt this time and not let it propagate by commenting out the next line? 
    309         Thread.currentThread().interrupt(); // re-interrupt the thread - which thread? Infinite loop? 
     353        log("*** Unexpected InterruptException when waiting for process stream gobblers to die:" + ie.getMessage(), ie); 
     354        } 
     355 
     356        // see comments in other runProcess() 
     357        Thread.currentThread().interrupt(); 
     358     
    310359    } finally {  
    311360 
    312         // Moved into here from GS2PerlConstructor which said 
    313         // "I need to somehow kill the child process. Unfortunately Thread.stop() and Process.destroy() both fail to do this. But now, thankx to the magic of Michaels 'close the stream suggestion', it works fine." 
    314         // http://steveliles.github.io/invoking_processes_from_java.html 
    315         // http://www.javaworld.com/article/2071275/core-java/when-runtime-exec---won-t.html?page=2 
    316         // http://mark.koli.ch/leaky-pipes-remember-to-close-your-streams-when-using-javas-runtimegetruntimeexec         
    317         if( prcs != null ) {         
    318         prcs.destroy(); 
    319         } 
     361        log("*** In finally of SafeProcess.runProcess(3 params)"); 
     362 
     363        if( process != null ) { 
     364        log("*** Going to call process.destroy 2"); 
     365        process.destroy(); 
     366        process = null; 
     367        log("*** Have called process.destroy 2"); 
     368        } 
     369         
    320370    } 
    321371     
     
    325375    public int runProcess(LineByLineHandler outLineByLineHandler, LineByLineHandler errLineByLineHandler) 
    326376    { 
    327     Process prcs = null; 
    328377    SafeProcess.OutputStreamGobbler inputGobbler = null; 
    329378    SafeProcess.InputStreamGobbler errorGobbler = null; 
     
    332381    try { 
    333382        // 1. get the Process object 
    334         prcs = doRuntimeExec(); 
     383        process = doRuntimeExec(); 
    335384         
    336385 
     
    340389        // send inputStr to process. The following constructor can handle inputStr being null 
    341390        inputGobbler = // WriterToProcessInputStream 
    342         new SafeProcess.OutputStreamGobbler(prcs.getOutputStream(), this.inputStr); 
     391        new SafeProcess.OutputStreamGobbler(process.getOutputStream(), this.inputStr); 
    343392 
    344393        // PROC ERR STREAM to monitor for any error messages or expected output in the process' stderr     
    345394        errorGobbler // ReaderFromProcessOutputStream 
    346             = new SafeProcess.InputStreamGobbler(prcs.getErrorStream(), splitStdErrorNewLines);      
     395            = new SafeProcess.InputStreamGobbler(process.getErrorStream(), splitStdErrorNewLines);       
    347396            // PROC OUT STREAM to monitor for the expected std output line(s) 
    348397        outputGobbler 
    349         = new SafeProcess.InputStreamGobbler(prcs.getInputStream(), splitStdOutputNewLines); 
     398        = new SafeProcess.InputStreamGobbler(process.getInputStream(), splitStdOutputNewLines); 
    350399 
    351400 
     
    360409 
    361410            // 4. kick off the stream gobblers 
    362         this.exitValue = waitForWithStreams(prcs, inputGobbler, outputGobbler, errorGobbler); 
     411        this.exitValue = waitForWithStreams(inputGobbler, outputGobbler, errorGobbler); 
    363412        
    364413    } catch(IOException ioe) { 
     
    366415        exceptionHandler.gotException(ioe); 
    367416        } else { 
    368         //logger.error("IOexception: " + ioe.getMessage(), ioe); 
    369         System.err.println("IOexception " + ioe.getMessage()); 
    370         ioe.printStackTrace(); 
    371         } 
    372     } catch(InterruptedException ie) { 
    373  
     417        log("IOexception: " + ioe.getMessage(), ioe);        
     418        } 
     419    } catch(InterruptedException ie) { // caused during any of the gobblers.join() calls, this is unexpected so log it 
     420         
    374421        if(exceptionHandler != null) { 
    375422        exceptionHandler.gotException(ie); 
     423        log("@@@@ Unexpected InterruptedException when waiting for process stream gobblers to die"); 
    376424        } else { 
    377         //logger.error("Process InterruptedException: " + ie.getMessage(), ie); 
    378         System.err.println("Process InterruptedException " + ie.getMessage()); 
    379         ///ie.printStackTrace(); // an interrupt here is not an error, it can be a cancel action 
    380         } 
    381  
    382         // propagate interrupts to worker threads here? 
    383         // unless the interrupt emanated from any of them in any join()... 
    384         // Only if the thread SafeProcess runs in was interrupted 
    385         // do we propagate the interrupt to the worker threads. 
    386         // http://stackoverflow.com/questions/2126997/who-is-calling-the-java-thread-interrupt-method-if-im-not 
    387         // "I know that in JCiP it is mentioned that you should never interrupt threads you do not own" 
    388         // But SafeProcess owns the worker threads, so it have every right to interrupt them 
    389         // Also read http://stackoverflow.com/questions/13623445/future-cancel-method-is-not-working?noredirect=1&lq=1 
    390         if(Thread.currentThread().isInterrupted()) { 
    391         inputGobbler.interrupt(); 
    392         errorGobbler.interrupt(); 
    393         outputGobbler.interrupt();       
    394         } 
    395  
    396         // On catchingInterruptedException, re-interrupt the thread. 
     425        log("*** Unexpected InterruptException when waiting for process stream gobblers to die: " + ie.getMessage(), ie); 
     426        } 
     427        // We're not causing any interruptions that may occur when trying to stop the worker threads 
     428        // So resort to default behaviour in this catch? 
     429        // "On catching InterruptedException, re-interrupt the thread." 
    397430        // This is just how InterruptedExceptions tend to be handled 
    398431        // See also http://stackoverflow.com/questions/4906799/why-invoke-thread-currentthread-interrupt-when-catch-any-interruptexception 
    399432        // and https://praveer09.github.io/technology/2015/12/06/understanding-thread-interruption-in-java/ 
    400  
    401         // http://stackoverflow.com/questions/3976344/handling-interruptedexception-in-java 
    402         // http://stackoverflow.com/questions/4906799/why-invoke-thread-currentthread-interrupt-when-catch-any-interruptexception 
    403         // "Only code that implements a thread's interruption policy may swallow an interruption request. General-purpose task and library code should never swallow interruption requests." 
    404         // Does that mean that since this code implements this thread's interruption policy, it's ok 
    405         // to swallow the interrupt this time and not let it propagate by commenting out the next line? 
    406433        Thread.currentThread().interrupt(); // re-interrupt the thread - which thread? Infinite loop? 
     434     
    407435    } finally {  
    408436 
    409         // Moved into here from GS2PerlConstructor which said 
    410         // "I need to somehow kill the child process. Unfortunately Thread.stop() and Process.destroy() both fail to do this. But now, thankx to the magic of Michaels 'close the stream suggestion', it works fine." 
     437        // Moved into here from GS2PerlConstructor and GShell.runLocal() which said 
     438        // "I need to somehow kill the child process. Unfortunately Thread.stop() and Process.destroy() both fail to do this. But now, thankx to the magic of Michaels 'close the stream suggestion', it works fine (no it doesn't!)" 
    411439        // http://steveliles.github.io/invoking_processes_from_java.html 
    412440        // http://www.javaworld.com/article/2071275/core-java/when-runtime-exec---won-t.html?page=2 
    413441        // http://mark.koli.ch/leaky-pipes-remember-to-close-your-streams-when-using-javas-runtimegetruntimeexec         
    414         if( prcs != null ) {         
    415         prcs.destroy(); 
    416         } 
     442        if( process != null ) { 
     443        log("*** Going to call process.destroy 1"); 
     444        process.destroy(); 
     445        process = null; 
     446        log("*** Have called process.destroy 1"); 
     447        } 
     448        log("*** In finally of SafeProcess.runProcess(2 params)"); 
    417449    } 
    418450     
     
    452484    protected CustomProcessHandler(int src) { 
    453485    this.source = src; // STDERR or STDOUT or STDIN 
     486 
     487    // modify threadname to prefix stream src (stdin/stderr/stdout) 
     488    // Useful for debugging if thread is named clearly 
     489    String stream; 
     490    switch(src) { 
     491    case SafeProcess.STDERR: 
     492       stream = "stderr";  
     493    case SafeProcess.STDOUT: 
     494        stream = "stdout"; 
     495    default: 
     496        stream = "stdin"; 
     497    } 
     498    Thread.currentThread().setName(stream + Thread.currentThread().getName()); 
    454499    } 
    455500     
     
    487532    public InputStreamGobbler(InputStream is) 
    488533    { 
     534    super("InputStreamGobbler"); // thread name 
    489535    this.is = is; 
    490536    this.split_newlines = false; 
     
    493539    public InputStreamGobbler(InputStream is, boolean split_newlines) 
    494540    { 
     541    super("InputStreamGobbler"); // thread name 
    495542    this.is = is; 
    496543    this.split_newlines = split_newlines; 
     
    499546    public InputStreamGobbler(InputStream is, CustomProcessHandler customHandler) 
    500547    { 
     548    super("InputStreamGobbler"); // thread name 
    501549    this.is = is; 
    502550    this.customHandler = customHandler; 
     
    514562        br = new BufferedReader(new InputStreamReader(is, "UTF-8")); 
    515563        String line=null; 
    516         while ( (line = br.readLine()) != null ) { 
    517  
    518         if(this.isInterrupted()) { // should we not instead check if SafeProcess thread was interrupted? 
    519             //logger.info("Got interrupted when reading lines from process err/out stream."); 
    520             //System.err.println("InputStreamGobbler.runDefault() Got interrupted when reading lines from process err/out stream."); 
    521             break; // will go to finally block 
    522         } 
    523  
    524         //System.out.println("@@@ GOT LINE: " + line); 
     564        while ( !this.isInterrupted() && (line = br.readLine()) != null ) { 
     565 
     566        //log("@@@ GOT LINE: " + line); 
    525567        outputstr.append(line); 
    526568        if(split_newlines) { 
     
    532574        } 
    533575        } 
     576 
    534577    } catch (IOException ioe) { 
    535578        if(lineByLineHandler != null) { 
    536579        lineByLineHandler.gotException(ioe); 
    537580        } else { 
    538         //logger.error("Exception when reading from a process' stdout/stderr stream: ", ioe); 
    539         System.err.println("Exception when reading from a process' stdout/stderr stream: "); 
    540         ioe.printStackTrace();   
     581        log("Exception when reading from a process' stdout/stderr stream: ", ioe); 
    541582        } 
    542583    } finally { 
     584        log("*********"); 
     585        if(this.isInterrupted()) { 
     586        log("We've been asked to stop.");        
     587        } 
    543588        SafeProcess.closeResource(br); 
     589        log("*** In finally of " + this.getName()); 
     590        log("*********"); 
    544591    } 
    545592    } 
     
    573620 
    574621    public OutputStreamGobbler(OutputStream os) { 
     622    super("OutputStreamGobbler"); // thread name 
    575623    this.os = os; 
    576624    } 
     
    578626    public OutputStreamGobbler(OutputStream os, String inputstr) 
    579627    { 
     628    super("OutputStreamGobbler"); // thread name 
    580629    this.os = os; 
    581630    this.inputstr = inputstr; 
     
    583632 
    584633    public OutputStreamGobbler(OutputStream os, CustomProcessHandler customHandler) { 
     634    super("OutputStreamGobbler"); // thread name 
    585635    this.os = os; 
    586636    this.customHandler = customHandler; 
     
    592642    if (inputstr == null) { 
    593643        return; 
     644    } 
     645 
     646    // also quit if the process was interrupted before we could send anything to its stdin 
     647    if(this.isInterrupted()) { 
     648        log(this.getName() + " thread was interrupted."); 
     649        return; 
    594650    } 
    595651     
     
    615671        */ 
    616672    } catch (IOException ioe) { 
    617         //logger.error("Exception writing to SafeProcess' inputstream: ", ioe); 
    618         System.err.println("Exception writing to SafeProcess' inputstream: "); 
    619         ioe.printStackTrace();   
     673        log("Exception writing to SafeProcess' inputstream: ", ioe); 
    620674    } finally { 
    621675        SafeProcess.closeResource(osw); 
     
    635689        runCustom(); 
    636690    } 
    637  
    638691    }    
    639692} // end static inner class OutputStreamGobbler 
     
    654707        } 
    655708    } catch(Exception e) { 
    656         //logger.error("Exception closing resource: ", e); 
    657         System.err.println("Exception closing resource: " + e.getMessage()); 
    658         e.printStackTrace(); 
     709        log("Exception closing resource: " + e.getMessage(), e); 
    659710        resourceHandle = null; 
    660711        success = false; 
     
    675726    } 
    676727 
     728// Moved from GShell.java 
     729    /** Determine if the given process is still executing. It does this by attempting to throw an exception - not the most efficient way, but the only one as far as I know 
     730     * @param process the Process to test 
     731     * @return true if it is still executing, false otherwise 
     732     */ 
     733    static public boolean processRunning(Process process) { 
     734    boolean process_running = false; 
     735 
     736    try { 
     737        process.exitValue(); // This will throw an exception if the process hasn't ended yet. 
     738    } 
     739    catch(IllegalThreadStateException itse) { 
     740        process_running = true; 
     741    } 
     742    catch(Exception exception) { 
     743        log(exception); // DebugStream.printStackTrace(exception); 
     744    } 
     745    return process_running; 
     746    } 
     747 
    677748} // end class SafeProcess