Changeset 31574

Show
Ignore:
Timestamp:
05.04.2017 20:22:15 (2 years ago)
Author:
ak19
Message:

Trying to correct the GS2PerlConstructor.runPerlCommand() function which has always been brittle, and causing a deadlocked Process when things go wrong. The current solution is with the new class SafeProcess?, which tries to properly create, run and end a process and manage its streams, and close them off. Have tried to ensure there will be no concurrency issues. Maybe it would be better to use Executor to instantiate threads (and to use its cancel abilities if we want this in future), but have stuck to existing code that worked. Changed GS2PerlConstructor to use this new SafeProcess? class in its new runPerlCommand() method. Tested document editing and adding user comments on Linux to find these things still work. However, the deadlock was happening on windows, so that's where the solution should be tested. Leaving in debug statements for windows testing.

Location:
main/trunk/greenstone3/src/java/org/greenstone
Files:
2 modified

Legend:

Unmodified
Added
Removed
  • main/trunk/greenstone3/src/java/org/greenstone/gsdl3/build/GS2PerlConstructor.java

    r30617 r31574  
    55import org.greenstone.util.Misc; 
    66import org.greenstone.util.GlobalProperties; 
     7import org.greenstone.util.SafeProcess; 
    78 
    89// xml classes 
     
    2728 * perl scripts to do the building stuff 
    2829 */ 
    29 public class GS2PerlConstructor extends CollectionConstructor 
     30public class GS2PerlConstructor extends CollectionConstructor implements SafeProcess.ExceptionHandler 
    3031{ 
    3132    static Logger logger = Logger.getLogger(org.greenstone.gsdl3.build.GS2PerlConstructor.class.getName()); 
     
    374375    return runPerlCommand(command, null, null); 
    375376    } 
    376      
    377     protected boolean runPerlCommand(String[] command, String[] envvars, File dir) 
     377 
     378         
     379    protected boolean runPerlCommand(String[] command, String[] envvars, File dir) 
     380    { 
     381    boolean success = true; 
     382     
     383    int sepIndex = this.gsdl3home.lastIndexOf(File.separator); 
     384    String srcHome = this.gsdl3home.substring(0, sepIndex); 
     385     
     386    ArrayList<String> args = new ArrayList<String>(); 
     387    args.add("GSDLHOME=" + this.gsdl2home); 
     388    args.add("GSDL3HOME=" + this.gsdl3home); 
     389    args.add("GSDL3SRCHOME=" + srcHome); 
     390    args.add("GSDLOS=" + this.gsdlos); 
     391    args.add("GSDL-RUN-SETUP=true"); 
     392    args.add("PERL_PERTURB_KEYS=0"); 
     393     
     394    if(envvars != null) { 
     395        for(int i = 0; i < envvars.length; i++) { 
     396        args.add(envvars[i]); 
     397        } 
     398    } 
     399     
     400    for (String a : System.getenv().keySet()) { 
     401        args.add(a + "=" + System.getenv(a)); 
     402    } 
     403     
     404    String command_str = ""; 
     405    for (int i = 0; i < command.length; i++) { 
     406        command_str = command_str + command[i] + " "; 
     407    } 
     408     
     409    sendMessage(new ConstructionEvent(this, GSStatus.INFO, "command = " + command_str)); 
     410     
     411     
     412    logger.info("### Running command = " + command_str); 
     413 
     414    // This is where we create and run our perl process safely 
     415    SafeProcess perlProcess  
     416        = new SafeProcess(command, args.toArray(new String[args.size()]), dir); //  dir can be null 
     417     
     418     
     419    sendProcessBegun(new ConstructionEvent(this, GSStatus.ACCEPTED, "starting")); 
     420     
     421    File logDir = new File(GSFile.collectDir(this.site_home) + File.separator + this.collection_name + File.separator + "log"); 
     422    if (!logDir.exists()) { 
     423        logDir.mkdir(); 
     424    } 
     425     
     426    // Only from Java 7+: Try-with-Resources block will safely close the BufferedWriter 
     427    // https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html 
     428    BufferedWriter bw = null; 
     429    try { 
     430        bw = new BufferedWriter(new FileWriter(GSFile.collectDir(this.site_home) + File.separator + this.collection_name + File.separator + "log" + File.separator + "build_log." + (System.currentTimeMillis()) + ".txt"));         
     431     
     432        bw.write("Document Editor Build \n");        
     433        bw.write("Command = " + command_str + "\n"); 
     434         
     435        // handle each incoming line from stdout and stderr streams, and any exceptions that occur then 
     436        SynchronizedProcessLineByLineHandler outLineByLineHandler 
     437        = new SynchronizedProcessLineByLineHandler(bw, SynchronizedProcessLineByLineHandler.STDOUT); 
     438        SynchronizedProcessLineByLineHandler errLineByLineHandler 
     439        = new SynchronizedProcessLineByLineHandler(bw, SynchronizedProcessLineByLineHandler.STDERR); 
     440        perlProcess.setStdOutLineByLineHandler(outLineByLineHandler); 
     441        perlProcess.setStdErrLineByLineHandler(errLineByLineHandler); 
     442         
     443        // GS2PerlConstructor will do further handling of exceptions that may occur during the perl 
     444        // process (including if writing something to the process' inputstream, not that we're doing that for this perlProcess) 
     445        perlProcess.setExceptionHandler(this);  
     446         
     447        // finally, execute the process 
     448         
     449        // Captures the std err of a program and pipes it into 
     450        // std in of java, as before. 
     451 
     452        logger.info("**** BEFORE runProcess."); 
     453        perlProcess.runProcess(); 
     454        logger.info("**** AFTER runProcess:"); 
     455         
     456    // The original runPerlCommand() code had an ineffective check for whether the cmd had been cancelled 
     457    // midway through executing the perl, as condition of a while loop reading from stderr and stdout. 
     458    // We don't include the cancel check here, as superclass CollectionConstructor.stopAction(), which set 
     459    // this.cancel to true, never got called anywhere. 
     460    // But I think a proper cancel of our perl process launched by this GS2PerlConstructor Thread object 
     461    // and of the worker threads it launches, could be implemented with interrupts. See: 
     462    // http://stackoverflow.com/questions/6859681/better-way-to-signal-other-thread-to-stop 
     463    // https://docs.oracle.com/javase/tutorial/essential/concurrency/interrupt.html 
     464    // https://docs.oracle.com/javase/7/docs/api/java/lang/Thread.html#interrupted() 
     465    // https://praveer09.github.io/technology/2015/12/06/understanding-thread-interruption-in-java/ 
     466    // The code that calls GS2PerlConstructor.stopAction() should also call GSPerlConstructor.interrupt() 
     467    // Then in SafeProcess.runProcess(), I think the waitFor() will throw an InterruptedException() 
     468    // This can be caught and interrupt() called on SafeProcess' workerthreads,  
     469    // Any workerthreads' run() methods that block (IO, loops) can test this.isInterrupted()  
     470    // and can break out of any loops and release resources in finally. 
     471    // Back in SafeProcess.runProcess, the InterruptedException catch block will be followed by finally 
     472    // that will clear up any further resources and destroy the process forcibly if it hadn't been ended. 
     473 
     474    } catch(IOException e) { 
     475        e.printStackTrace(); 
     476        sendProcessStatus(new ConstructionEvent(this, GSStatus.ERROR, "Exception occurred " + e.toString())); 
     477    } finally { 
     478        SafeProcess.closeResource(bw); 
     479    } 
     480 
     481    if (!this.cancel) { 
     482        // Now display final message based on exit value             
     483         
     484        if (perlProcess.getExitValue() == 0) {   
     485        //status = OK; 
     486        sendProcessStatus(new ConstructionEvent(this, GSStatus.CONTINUING, "Success")); 
     487         
     488        success = true; 
     489        } else { 
     490        //status = ERROR; 
     491        sendProcessStatus(new ConstructionEvent(this, GSStatus.ERROR, "Failure")); 
     492         
     493        //return false; 
     494        success = false; 
     495         
     496        } 
     497    } else { // cancelled 
     498         
     499        // 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. 
     500        sendProcessStatus(new ConstructionEvent(this, GSStatus.HALTED, "killing the process")); 
     501        //prcs.getOutputStream().close(); 
     502        //prcs.destroy(); 
     503        ////status = ERROR; 
     504         
     505        //return false; 
     506        success = false; 
     507    } 
     508    // we're done, but we don't send a process complete message here cos there might be stuff to do after this has finished. 
     509    //return true;       
     510    return success; 
     511    } 
     512     
     513 
     514    // this method is blocking again, on Windows when adding user comments 
     515    protected boolean old_runPerlCommand(String[] command, String[] envvars, File dir) 
    378516    { 
    379517        boolean success = true; 
     
    406544            command_str = command_str + command[i] + " "; 
    407545        } 
     546 
     547logger.info("### old runPerlCmd, command = " + command_str); 
    408548 
    409549        sendMessage(new ConstructionEvent(this, GSStatus.INFO, "command = " + command_str)); 
     
    454594                } 
    455595            } 
    456             closeResource(bw);  
     596            SafeProcess.closeResource(bw);  
    457597             
    458598            if (!this.cancel) 
     
    500640         
    501641             if( prcs != null ) { 
    502                 closeResource(prcs.getErrorStream()); 
    503                 closeResource(prcs.getOutputStream()); 
    504                 closeResource(prcs.getInputStream()); 
     642                SafeProcess.closeResource(prcs.getErrorStream()); 
     643                SafeProcess.closeResource(prcs.getOutputStream()); 
     644                SafeProcess.closeResource(prcs.getInputStream()); 
    505645                prcs.destroy(); 
    506646            } 
    507647         
    508             closeResource(ebr); 
    509             closeResource(stdinbr); 
     648            SafeProcess.closeResource(ebr); 
     649            SafeProcess.closeResource(stdinbr); 
    510650        } 
    511651 
     
    514654        return success; 
    515655    } 
    516      
    517     public static void closeResource(Closeable resourceHandle) { 
    518         try { 
    519             if(resourceHandle != null) { 
    520                 resourceHandle.close(); 
    521                 resourceHandle = null; 
    522             } 
    523         } catch(Exception e) { 
    524             System.err.println("Exception closing resource: " + e.getMessage()); 
    525             e.printStackTrace(); 
    526         } 
     656 
     657 
     658    // From interface SafeProcess.ExceptionHandler 
     659    // Called when an exception happens during the running of our perl process. However, 
     660    // exceptions when reading from our perl process' stderr and stdout streams are handled by 
     661    // SynchronizedProcessLineByLineHandler.gotException() below. 
     662    public void gotException(Exception e) { 
     663 
     664    // do what original runPerlCommand() code always did when an exception occurred 
     665    // when running the perl process: 
     666    e.printStackTrace(); 
     667    sendProcessStatus(new ConstructionEvent(this,GSStatus.ERROR,  
     668                        "Exception occurred " + e.toString())); // atomic 
    527669    } 
     670     
     671    // This class deals with each incoming line from the perl process' stderr or stdout streams. One 
     672    // instance of this class for each stream. However, since multiple instances of this LineByLineHandler  
     673    // could be (and in fact, are) writing to the same file in their own threads, the writer object needs  
     674    // to be made threadsafe. 
     675    // This class also handles exceptions during the running of the perl process. 
     676    // The runPerlCommand code originally would do a sendProcessStatus on each exception, so we ensure 
     677    // we do that here too, to continue original behaviour. 
     678    protected class SynchronizedProcessLineByLineHandler implements SafeProcess.LineByLineHandler 
     679    { 
     680    public static final int STDERR = 0; 
     681    public static final int STDOUT = 1; 
     682 
     683    private final int source; 
     684    private final BufferedWriter bwHandle; // needs to be final to synchronize on the object 
     685         
     686 
     687    public SynchronizedProcessLineByLineHandler(BufferedWriter bw, int src) { 
     688        this.bwHandle = bw; 
     689        this.source = src; // STDERR or STDOUT 
     690    } 
     691 
     692    public void gotLine(String line) { 
     693        //if(this.source == STDERR) { 
     694        ///System.err.println("ERROR: " + line); 
     695        //} else { 
     696        ///System.err.println("OUT: " + line); 
     697        //} 
     698 
     699        // BufferedWriter writes may not be atomic 
     700        // http://stackoverflow.com/questions/9512433/is-writer-an-atomic-method 
     701        // Choosing to put try-catch outside of sync block, since it's okay to give up lock on exception 
     702        // http://stackoverflow.com/questions/14944551/it-is-better-to-have-a-synchronized-block-inside-a-try-block-or-a-try-block-insi 
     703        // "All methods on Logger are multi-thread safe", see 
     704        // http://stackoverflow.com/questions/14211629/java-util-logger-write-synchronization 
     705 
     706        try { 
     707        synchronized(bwHandle) { // get a lock on the writer handle, then write 
     708         
     709            bwHandle.write(line + "\n"); 
     710        }  
     711        } catch(IOException ioe) { 
     712        String msg = (source == STDERR) ? "stderr" : "stdout"; 
     713        msg = "Exception when writing out a line read from perl process' " + msg + " stream."; 
     714        GS2PerlConstructor.logger.error(msg, ioe); 
     715        } 
     716         
     717        // this next method is thread safe since only synchronized methods are invoked. 
     718        // and only immutable (final) vars are used. 
     719        sendProcessStatus(new ConstructionEvent(GS2PerlConstructor.this, GSStatus.CONTINUING, line)); 
     720    }    
     721 
     722    // This is called when we get an exception during the processing of a perl's 
     723    // input-, err- or output stream 
     724    public void gotException(Exception e) { 
     725        String msg = (source == STDERR) ? "stderr" : "stdout"; 
     726        msg = "Got exception when processing the perl process' " + msg + " stream."; 
     727 
     728        // now do what the original runPerlCommand() code always did: 
     729        e.printStackTrace(); 
     730        sendProcessStatus(new ConstructionEvent(this, GSStatus.ERROR, "Exception occurred " + e.toString())); // atomic 
     731    } 
     732 
     733    } // end inner class SynchronizedProcessLineByLineHandler 
     734 
    528735} 
  • main/trunk/greenstone3/src/java/org/greenstone/util/SafeProcess.java

    r31569 r31574  
    2323    static Logger logger = Logger.getLogger(org.greenstone.util.SafeProcess.class.getName()); 
    2424 
    25     private String[] command_args; 
     25    private String[] command_args = null; 
     26    private String[] envp = null; 
     27    private File dir = null; 
    2628    private String inputStr = null; 
    2729 
     
    2931    private String errorStr = ""; 
    3032 
    31     private int process_exitValue = -1; 
    32     private ProcessLineByLineHandler lineByLineHandler = null; 
     33    private int exitValue = -1; 
     34 
     35    // user can write custom LineByLineHandler to deal with stdout lines as they come out one line at a time 
     36    // and stderr lines as they come out one at a time 
     37    private LineByLineHandler errLineByLineHandler = null; 
     38    private LineByLineHandler outLineByLineHandler = null; 
     39    private ExceptionHandler exceptionHandler = null; 
    3340 
    3441    // whether std/err output should be split at new lines 
     
    3744 
    3845    // call one of these constructors 
     46 
     47    // cmd args version 
    3948    public SafeProcess(String[] cmd_args) 
    4049    { 
    41     this(cmd_args, null, null, false, false); 
    42     } 
    43  
    44     public SafeProcess(String[] cmd_args, ProcessLineByLineHandler lbl_handler) 
    45     { 
    46     this(cmd_args, lbl_handler, null, false, false); 
    47     } 
    48  
    49     public SafeProcess(String[] cmd_args, ProcessLineByLineHandler lbl_handler, 
    50                String sendStr, boolean splitStdOut, boolean splitStdErr) 
    51     {    
    5250    command_args = cmd_args; 
    53     lineByLineHandler = lbl_handler; 
    54     setInputString(sendStr); 
    55     setSplitStdOutputNewLines(splitStdOut); 
    56     setSplitStdErrorNewLines(splitStdErr); 
    57     runProcess(); 
     51    } 
     52 
     53    // cmd args with env version, launchDir can be null. 
     54    public SafeProcess(String[] cmd_args, String[] envparams, File launchDir) 
     55    { 
     56    command_args = cmd_args; 
     57    envp = envparams; 
     58    dir = launchDir; 
    5859    } 
    5960 
     
    6263    public String getStdOutput() { return outputStr; } 
    6364    public String getStdError() { return errorStr; } 
    64     public int getExitValue() { return process_exitValue; } 
    65  
     65    public int getExitValue() { return exitValue; } 
     66 
     67     
     68    // set any string to send as input to the process spawned by SafeProcess 
    6669    public void setInputString(String sendStr) { 
    6770    inputStr = sendStr; 
    6871    } 
     72 
     73    // register a handler whose gotLine() method will get called as each line is read from the process' stdout 
     74    public void setStdOutLineByLineHandler(LineByLineHandler out_lbl_handler) { 
     75    outLineByLineHandler = out_lbl_handler; 
     76    } 
     77 
     78    // register a handler whose gotLine() method will get called as each line is read from the process' stderr 
     79    public void setStdErrLineByLineHandler(LineByLineHandler err_lbl_handler) { 
     80    errLineByLineHandler = err_lbl_handler; 
     81    } 
     82 
     83    // register a SafeProcess ExceptionHandler whose gotException() method will 
     84    // get called for each exception encountered 
     85    public void setExceptionHandler(ExceptionHandler exception_handler) { 
     86    exceptionHandler = exception_handler;    
     87    } 
     88 
     89    // set if you want the std output or err output to have \n at each newline read from the stream 
    6990    public void setSplitStdOutputNewLines(boolean split) { 
    7091    splitStdOutputNewLines = split; 
     
    7596 
    7697//***************** Copied from gli's gui/FormatConversionDialog.java *************// 
    77     private void runProcess() { 
     98    public void runProcess() { 
     99    Process prcs = null; 
     100    SafeProcess.OutputStreamGobbler inputGobbler = null; 
     101    SafeProcess.InputStreamGobbler errorGobbler = null; 
     102    SafeProcess.InputStreamGobbler outputGobbler = null; 
    78103 
    79104    try {        
    80105         
    81         Runtime rt = Runtime.getRuntime(); 
    82         Process prcs = null; 
     106        Runtime rt = Runtime.getRuntime();       
     107         
    83108         
    84109        // http://stackoverflow.com/questions/5283444/convert-array-of-strings-into-a-string-in-java 
    85110        logger.info("Running process: " + Arrays.toString(command_args)); 
    86         prcs = rt.exec(this.command_args); 
    87  
     111 
     112        if(this.envp == null) { 
     113        prcs = rt.exec(this.command_args); 
     114        } else { // launch process using cmd str with env params         
     115 
     116        if(this.dir == null) { 
     117            logger.info("\twith: " + Arrays.toString(this.envp)); 
     118            prcs = rt.exec(this.command_args, this.envp); 
     119        } else { 
     120            logger.info("\tfrom directory: " + this.dir); 
     121            logger.info("\twith: " + Arrays.toString(this.envp)); 
     122            prcs = rt.exec(this.command_args, this.envp, this.dir); 
     123        } 
     124        } 
     125 
     126        logger.info("### Before creating ProcessInGobbler"); 
     127 
     128        
    88129        // send inputStr to process. The following constructor can handle inputStr being null 
    89         SafeProcess.OutputStreamGobbler inputGobbler =  
     130        inputGobbler = // WriterToProcessInputStream 
    90131        new SafeProcess.OutputStreamGobbler(prcs.getOutputStream(), this.inputStr); 
    91132         
     133        logger.info("### Before creating ProcessErrGobbler"); 
     134         
    92135        // monitor for any error messages 
    93             SafeProcess.InputStreamGobbler errorGobbler  
     136            errorGobbler // ReaderFromProcessOutputStream 
    94137        = new SafeProcess.InputStreamGobbler(prcs.getErrorStream(), splitStdOutputNewLines); 
    95              
     138 
     139        logger.info("### Before creating ProcessOutGobbler"); 
     140 
    96141            // monitor for the expected std output line(s) 
    97             SafeProcess.InputStreamGobbler outputGobbler 
     142            outputGobbler 
    98143        = new SafeProcess.InputStreamGobbler(prcs.getInputStream(), splitStdErrorNewLines); 
    99144 
    100             // kick them off 
     145        logger.info("### Before setting handlers on ProcessGobblers"); 
     146                     
     147        // register line by line handlers, if any were set, for the process stderr and stdout streams 
     148        if(this.outLineByLineHandler != null) { 
     149        outputGobbler.setLineByLineHandler(this.outLineByLineHandler); 
     150        } 
     151        if(this.errLineByLineHandler != null) { 
     152        errorGobbler.setLineByLineHandler(this.errLineByLineHandler); 
     153        } 
     154        if(this.exceptionHandler != null) { 
     155        inputGobbler.setExceptionHandler(this.exceptionHandler); 
     156        }        
     157 
     158        logger.info("### Before streamgobblers.start()"); 
     159 
     160            // kick off the stream gobblers 
    101161            inputGobbler.start(); 
    102162            errorGobbler.start(); 
    103163            outputGobbler.start(); 
     164 
     165        logger.info("### After streamgobblers.start() - before waitFor"); 
    104166                                     
    105167            // any error??? 
    106             this.process_exitValue = prcs.waitFor(); 
    107             //System.out.println("ExitValue: " + exitVal);  
     168            this.exitValue = prcs.waitFor(); // can throw an InterruptedException if process did not terminate               
     169            logger.info("ExitValue: " + exitValue);  
     170 
     171        logger.info("### Before streamgobblers.join()"); 
    108172 
    109173        // From the comments of  
    110174        // http://www.javaworld.com/article/2071275/core-java/when-runtime-exec---won-t.html?page=2 
    111175        // To avoid running into nondeterministic failures to get the process output 
    112         // if there's no waiting for the threads, call join() on each Thread (StreamGobbler) object: 
    113         outputGobbler.join(); 
     176        // if there's no waiting for the threads, call join() on each Thread (StreamGobbler) object. 
     177        // From Thread API: join() "Waits for this thread (the thread join() is invoked on) to die." 
     178        outputGobbler.join();  
    114179        errorGobbler.join(); 
    115         inputGobbler.join(); 
    116          
     180        inputGobbler.join();  
     181         
     182        logger.info("### After streamgobblers.join()"); 
     183 
    117184        // set the variables the code that created a SafeProcess object may want to inspect 
    118185        this.outputStr = outputGobbler.getOutput(); 
     
    120187 
    121188        // the calling code should handle errorStr, not us, so can leave out the following code 
    122         if(!this.errorStr.equals("")) { 
    123         logger.info("*** Process errorstream: \n" + this.errorStr + "\n****"); 
    124         System.err.println("*** Process errorstream: \n" + this.errorStr + "\n****"); 
    125         } 
     189        /* 
     190          if(!this.errorStr.equals("")) { 
     191          logger.info("*** Process errorstream: \n" + this.errorStr + "\n****"); 
     192          //System.err.println("*** Process errorstream: \n" + this.errorStr + "\n****"); 
     193          } 
     194        */ 
     195         
     196        // Since we didn't have an exception, process should have terminated now (waitFor blocks until then) 
     197        // Set process to null so we don't forcibly terminate it below with process.destroy() 
     198        prcs = null; 
    126199        
    127200    } catch(IOException ioe) { 
    128         logger.error("IOexception: " + ioe.getMessage()); 
    129         System.err.println("IOexception " + ioe.getMessage()); 
    130         ioe.printStackTrace(); 
     201        logger.error("IOexception: " + ioe.getMessage(), ioe); 
     202        //System.err.println("IOexception " + ioe.getMessage()); 
     203        //ioe.printStackTrace(); 
     204        if(exceptionHandler != null) exceptionHandler.gotException(ioe); 
    131205    } catch(InterruptedException ie) { 
    132         logger.error("Process InterruptedException: " + ie.getMessage()); 
    133         System.err.println("Process InterruptedException " + ie.getMessage()); 
    134         ie.printStackTrace(); 
    135     } 
    136  
    137     } 
    138  
    139      
    140  
    141 //**************** Inner class definitions copied from GLI **********// 
     206        logger.error("Process InterruptedException: " + ie.getMessage(), ie); 
     207        //System.err.println("Process InterruptedException " + ie.getMessage()); 
     208        //ie.printStackTrace(); 
     209        if(exceptionHandler != null) exceptionHandler.gotException(ie); 
     210 
     211        // propagate interrupts to worker threads here? 
     212        // unless the interrupt emanated from any of them in any join()... 
     213        // Only if the thread SafeProcess runs in was interrupted 
     214        // do we propagate the interrupt to the worker threads. 
     215        // http://stackoverflow.com/questions/2126997/who-is-calling-the-java-thread-interrupt-method-if-im-not 
     216        // "I know that in JCiP it is mentioned that you should never interrupt threads you do not own" 
     217        // But SafeProcess owns the worker threads, so it have every right to interrupt them 
     218        // Also read http://stackoverflow.com/questions/13623445/future-cancel-method-is-not-working?noredirect=1&lq=1 
     219        if(Thread.currentThread().isInterrupted()) { 
     220        inputGobbler.interrupt(); 
     221        errorGobbler.interrupt(); 
     222        outputGobbler.interrupt();       
     223        } 
     224 
     225        // On catchingInterruptedException, re-interrupt the thread. 
     226        // This is just how InterruptedExceptions tend to be handled 
     227        // See also http://stackoverflow.com/questions/4906799/why-invoke-thread-currentthread-interrupt-when-catch-any-interruptexception 
     228        // and https://praveer09.github.io/technology/2015/12/06/understanding-thread-interruption-in-java/ 
     229 
     230        // http://stackoverflow.com/questions/3976344/handling-interruptedexception-in-java 
     231        // http://stackoverflow.com/questions/4906799/why-invoke-thread-currentthread-interrupt-when-catch-any-interruptexception 
     232        // "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." 
     233        // Does that mean that since this code implements this thread's interruption policy, it's ok 
     234        // to swallow the interrupt this time and not let it propagate by commenting out the next line? 
     235        Thread.currentThread().interrupt(); // re-interrupt the thread - which thread? Infinite loop? 
     236    } finally {  
     237 
     238        // Moved into here from GS2PerlConstructor which said 
     239        // "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." 
     240        // http://steveliles.github.io/invoking_processes_from_java.html 
     241        // http://www.javaworld.com/article/2071275/core-java/when-runtime-exec---won-t.html?page=2 
     242        // http://mark.koli.ch/leaky-pipes-remember-to-close-your-streams-when-using-javas-runtimegetruntimeexec         
     243        if( prcs != null ) {         
     244        prcs.destroy(); 
     245        } 
     246    } 
     247 
     248    } 
     249     
     250 
     251//**************** Inner class definitions (stream gobblers copied from GLI) **********// 
    142252// Static inner classes can be instantiated without having to instantiate an object of the outer class first 
    143253 
    144      
     254// Can have public static interfaces too, 
     255// see http://stackoverflow.com/questions/71625/why-would-a-static-nested-interface-be-used-in-java 
     256// Implementors need to take care that the implementations are thread safe 
     257// http://stackoverflow.com/questions/14520814/why-synchronized-method-is-not-included-in-interface 
     258public static interface ExceptionHandler { 
     259 
     260    // SHOULD I DECLARE THIS SYNCHRONIZED? 
     261    // It ends up being thread safe for the particular instance I'm using it for, but that doesn't 
     262    // make it future proof... 
     263    public void gotException(Exception e); 
     264} 
     265 
    145266// When reading from a process' stdout or stderr stream, you can create a LineByLineHandler 
    146267// to do something on a line by line basis, such as sending the line to a log 
    147 // Can have public static interfaces too, 
    148 // see http://stackoverflow.com/questions/71625/why-would-a-static-nested-interface-be-used-in-java 
    149 public static interface ProcessLineByLineHandler { 
    150  
     268public static interface LineByLineHandler { 
    151269    public void gotLine(String line); 
     270    public void gotException(Exception e); // for when an exception occurs instead of getting a line 
    152271} 
     272 
    153273 
    154274// http://www.javaworld.com/article/2071275/core-java/when-runtime-exec---won-t.html?page=2 
     
    160280    StringBuffer outputstr = new StringBuffer(); 
    161281    boolean split_newlines = false; 
    162     ProcessLineByLineHandler lineByLineHandler = null; 
     282    LineByLineHandler lineByLineHandler = null; 
    163283     
    164284    public InputStreamGobbler(InputStream is) 
     
    174294    } 
    175295     
    176     public void setLineByLineHandler(ProcessLineByLineHandler lblHandler) { 
     296    public void setLineByLineHandler(LineByLineHandler lblHandler) { 
    177297    lineByLineHandler = lblHandler; 
    178298    } 
     299 
    179300 
    180301    public void run() 
     
    184305        br = new BufferedReader(new InputStreamReader(is, "UTF-8")); 
    185306        String line=null; 
    186         while ( (line = br.readLine()) != null) { 
     307        while ( (line = br.readLine()) != null ) { 
     308 
     309        if(this.isInterrupted()) { // should we not instead check if SafeProcess thread was interrupted? 
     310            logger.info("Got interrupted when reading lines from process err/out stream."); 
     311            break; // will go to finally block 
     312        } 
     313 
    187314        //System.out.println("@@@ GOT LINE: " + line); 
    188315        outputstr.append(line); 
    189316        if(split_newlines) { 
    190             outputstr.append("\n"); 
     317            outputstr.append(Misc.NEWLINE); // "\n" is system dependent (Win must be "\r\n") 
    191318        } 
    192319 
    193320        if(lineByLineHandler != null) { // let handler deal with newlines 
    194321            lineByLineHandler.gotLine(line); 
    195         } 
    196          
     322        }        
    197323        } 
    198324    } catch (IOException ioe) { 
    199         ioe.printStackTrace();   
     325        logger.error("Exception when reading from a process' stdout/stderr stream: ", ioe); 
     326        if(lineByLineHandler != null) lineByLineHandler.gotException(ioe); 
     327        //ioe.printStackTrace();   
    200328    } finally { 
    201329        SafeProcess.closeResource(br); 
     
    212340// This class is used in FormatConversionDialog to properly write to the inputstream of a Process 
    213341// Process.getOutputStream() 
    214 public class OutputStreamGobbler extends Thread 
     342public static class OutputStreamGobbler extends Thread 
    215343{ 
    216344    OutputStream os = null; 
    217345    String inputstr = ""; 
    218      
     346    ExceptionHandler exceptionHandler = null; 
     347 
    219348    public OutputStreamGobbler(OutputStream os, String inputstr) 
    220349    { 
     
    223352    } 
    224353     
     354    public void setExceptionHandler(ExceptionHandler eHandler) { 
     355    exceptionHandler = eHandler; 
     356    } 
     357 
    225358    public void run() 
    226     { 
    227      
     359    {    
    228360    if (inputstr == null) { 
    229361        return; 
     
    251383        */ 
    252384    } catch (IOException ioe) { 
    253         ioe.printStackTrace();   
     385        logger.error("Exception writing to SafeProcess' inputstream: ", ioe);        
     386        //ioe.printStackTrace(); 
     387 
     388        if (this.exceptionHandler != null) this.exceptionHandler.gotException(ioe); 
     389         
    254390    } finally { 
    255391        SafeProcess.closeResource(osw); 
     
    264400    // http://docs.oracle.com/javase/tutorial/essential/exceptions/finally.html 
    265401    // http://stackoverflow.com/questions/481446/throws-exception-in-finally-blocks 
    266     public static void closeResource(Closeable resourceHandle) { 
     402    public static boolean closeResource(Closeable resourceHandle) { 
     403    boolean success = false; 
    267404    try { 
    268405        if(resourceHandle != null) { 
    269406        resourceHandle.close(); 
    270407        resourceHandle = null; 
     408        success = true; 
    271409        } 
    272410    } catch(Exception e) { 
    273         System.err.println("Exception closing resource: " + e.getMessage()); 
    274         e.printStackTrace(); 
     411        logger.error("Exception closing resource: ", e); 
     412        //System.err.println("Exception closing resource: " + e.getMessage()); 
     413        //e.printStackTrace(); 
    275414        resourceHandle = null; 
    276     } 
    277     } 
    278  
    279     public static void closeProcess(Process prcs) { 
     415        success = false; 
     416    } finally { 
     417        return success; 
     418    } 
     419    } 
     420 
     421    public static boolean closeProcess(Process prcs) { 
     422    boolean success = true; 
    280423    if( prcs != null ) {  
    281         closeResource(prcs.getErrorStream());  
    282         closeResource(prcs.getOutputStream());  
    283         closeResource(prcs.getInputStream());  
     424        success = success && closeResource(prcs.getErrorStream());  
     425        success = success && closeResource(prcs.getOutputStream());  
     426        success = success && closeResource(prcs.getInputStream());  
    284427        prcs.destroy();  
    285     }  
     428    } 
     429    return success; 
    286430    } 
    287431