Index: /main/trunk/gli/src/org/greenstone/gatherer/util/Readme_Using_SafeProcess.txt
===================================================================
--- /main/trunk/gli/src/org/greenstone/gatherer/util/Readme_Using_SafeProcess.txt	(revision 32321)
+++ /main/trunk/gli/src/org/greenstone/gatherer/util/Readme_Using_SafeProcess.txt	(revision 32322)
@@ -30,4 +30,7 @@
 	N. GLI vs GS3 CODE's SAFEPROCESS.JAVA - THE DIFFERENCES
 	O. FUTURE WORK, IMPROVEMENTS
+	P. ALTERNATIVE FUTURE WORK: A case for rewriting SafeProcess to use IO::Select (Selector in Java) instead of multi-threading
+	Q. PUBLIC API OF SAFEPROCESS, ITS SUB CLASSES AND INTERFACES
+
 
 _______________________________________________________________________________
@@ -926,7 +929,8 @@
 _____________________________________________________________________________________________________________________
 
-P. STREAM GOBBLER **THREADS** ARE ALLOWED TO BLOCK (JVM WILL TIMESLICE). CAUSE INTERRUPTEDEXCEPTIONS TO END BLOCKS
+P. ALTERNATIVE FUTURE WORK: A case for rewriting SafeProcess to use IO::Select (Selector in Java) instead of multi-threading
 _____________________________________________________________________________________________________________________
 
+(a) STREAM GOBBLER **THREADS** ARE ALLOWED TO BLOCK (JVM WILL TIMESLICE). CAUSE INTERRUPTEDEXCEPTIONS TO END BLOCKS
 In the InputStreamGobbler Thread classes for handling the proc.errstream and proc.outstream, the run() methods have a loop on readLine():
 
@@ -942,4 +946,5 @@
      // So must check BufferedReader.ready() before attempting readLine() on it
      // See https://stackoverflow.com/questions/15521352/bufferedreader-readline-blocks
+     // (also http://www.java-gaming.org/index.php?topic=37191.0)
      while ( !this.isInterrupted() && br.ready() && (line = br.readLine()) != null ) { ...
 
@@ -981,7 +986,7 @@
 
 
-ANOTHER IMPORTANT LESSON FROM ANDREW:
+(b) ANOTHER IMPORTANT LESSON FROM ANDREW:
 Andrew Mackintosh further said that the read() block calls in worker Threads are far better in terms of not wasting processor power than a constantly active while loop in the worker Thread.
-1. So doing this:
+1. So doing something like this:
 
     while(!thisthread.isInterrupted() && br.read() ) // br.read() possibly blocks
@@ -1003,4 +1008,166 @@
 Other links:
 - EOF/EOS (End of Stream) is indicated by a return value of -1 for read and null for readLine().
-https://stackoverflow.com/questions/36569875/how-does-bufferedreader-readline-handle-eof-or-slow-input
-https://stackoverflow.com/questions/3714090/how-to-see-if-a-reader-is-at-eof
+- https://stackoverflow.com/questions/36569875/how-does-bufferedreader-readline-handle-eof-or-slow-input
+- https://stackoverflow.com/questions/3714090/how-to-see-if-a-reader-is-at-eof
+- https://arstechnica.com/civis/viewtopic.php?t=259678
+
+(c) I next tried reading a char at a time by calling BufferedReader.read(), but it blocked forever. With further testing, I confirmed that for the indefinitely blocking process stream (stdout in the use case where things went wrong), the bufferedreader and underlying stream is never ready().
+
+Andrew then pointed me to the API for the BufferedReader.read(cbuf) variant at https://docs.oracle.com/javase/7/docs/api/java/io/BufferedReader.html#read(char[],%20int,%20int), which like the read() variant would return -1 on eof/eos. The API for read(cbuf) claimed that it would return the number of bytes read, -1 if eof/eos or return when ready() returns false. The latter was not true, and this overloaded method behaved the same as the read() variant that reads one char at a time: the buffer and underlying stream were never ready() and hence all variants of BufferedReader's reading methods always blocked.
+
+Following advice at https://www.experts-exchange.com/questions/20083639/BufferedReader-not-ready.html and elsewhere, I tried reading directly from InputStream a.o.t. going through BufferedReader, and tested the InputStream.available() (equivalent of ready()) before attempting to read() from the stream, 
+
+- https://docs.oracle.com/javase/7/docs/api/java/io/InputStream.html#available()
+- https://stackoverflow.com/questions/3695372/what-does-inputstream-available-do-in-java
+
+(Further, the example of InterruptibleReadline at https://stackoverflow.com/questions/3595926/how-to-interrupt-bufferedreaders-readline is buggy and doesn't work when corrected: it's the same problem.)
+
+
+I finally came to the realisation our threaded SafeProcess would never work, and the references on the web to using java.nio when you want to do non-blocking IO started to make sense.
+I had considered and attempted setting a timeout to help detect when a stream might block forever, versus when a stream would eventually get data. The classes of java.nio were again recommended when you want to work with timeouts during IO operations.
+
+
+(d) java.nio - stands for New IO
+Thinking it stood for non-blocking IO, I googled for:
+   java nio non blocking io
+to confirm whether this was so, and not that nio stood for "native IO". The first search result was:
+
+"Blocking vs. Non-blocking IO. Java IO's various streams are blocking. That means, that when a thread invokes a read() or write() , that thread is blocked until there is some data to read, or the data is fully written. The thread can do nothing else in the meantime. Jun 23, 2014"
+Java NIO vs. IO - Jenkov Tutorials
+tutorials.jenkov.com/java-nio/nio-vs-io.html"
+
+If only I had found information like this earlier. However, I may not have understood it or its significance at the time (before working on NetStinky).
+- https://www.experts-exchange.com/questions/20083639/BufferedReader-not-ready.html
+- https://www.quora.com/How-can-I-get-BufferedReader-to-only-wait-for-an-input-from-System-in-for-a-given-number-of-milliseconds
+
+There is a Selector class in Java.nio which works like perl and C++ IO::Select().
+
+
+Tutorials and further pages to read:
+- http://tutorials.jenkov.com/java-nio/nio-vs-io.html (general comparison and when to use java.io vs java.nio)
+- http://www.baeldung.com/java-nio-selector (the most useful for us)
+
+
+Bookmarked:
+- https://stackoverflow.com/questions/6619516/using-filechannel-to-write-any-inputstream
+says that to get a channel for an inputstream, you do:
+	Channels.newChannel(InputStream in)
+	http://docs.oracle.com/javase/7/docs/api/java/nio/channels/Channels.html
+- More information on channels: http://www.java2s.com/Tutorials/Java/Java_io/0930__Java_nio_Channels.htm	
+- Different kind of buffers: http://www.ntu.edu.sg/home/ehchua/programming/java/J5b_IO_advanced.html
+- https://docs.oracle.com/javase/7/docs/api/java/nio/channels/InterruptibleChannel.html (we want to use interruptible channels, so our channel must implement this)
+- https://docs.oracle.com/javase/7/docs/api/java/nio/channels/spi/AbstractSelectableChannel.html (we want to add our channel to a select, so our channel must implement this)
+- https://docs.oracle.com/javase/7/docs/api/java/nio/channels/Channels.html (factory for obtaining a channel)
+- https://docs.oracle.com/javase/7/docs/api/java/nio/channels/ReadableByteChannel.html (object returned by doing Channels.newChannel(InputStream in) is of type ReadableByteChannel)
+
+I think Selector is the way to go to reimplement SafeProcess using Select to do non-blocking IO (with built in timeouts) in place of multi-threading. Therefore read through http://www.baeldung.com/java-nio-selector to refresh memory on behaviour of IO::select() in general and learn about how to do it in Java in particular.
+
+The idea is to move the multi-threaded SafeProcess to SafeProcess2.java and re-write SafeProces.java to use Java's IO::Select() (Selector). Then the classes that use SafeProcess would make the same calls as before. If we ever do go this route, see Section Q below for the public API of SafeProcess that needs to be reimplemented with java.nio/Selector.
+
+
+General information:
+https://stackoverflow.com/questions/355089/difference-between-stringbuilder-and-stringbuffer
+
+
+(e) Dr Bainbridge fixed the problem at the source: in the OpenOfficeConverter.pm which was launching soffice in headless mode with the wrong command: when 2>&1 is further redirected to a file (or somewhere else like /dev/null), need to do the file redirect first and then 2>&1.
+
+So, "cmd >file 2>&1" is the right way, whereas "cmd 2>&1 >file" is the wrong way around. OpenOfficeConverter.pm was launching soffice the wrong way around and Dr Bainbridge fixed it.
+
+He says we don't need to pursue rewriting SafeProcess to use Selector, as the issue in GLI was ultimately due to an error in the command that was launched in perl. However, Dr Bainbridge said it was OK to commit the links I've found on using Selector and a description of the issue that led to considering Selector.
+
+
+
+_____________________________________________________________
+Q. PUBLIC API OF SAFEPROCESS, ITS SUB CLASSES AND INTERFACES
+_____________________________________________________________
+WE ONLY CARE ABOUT NON-GOBBLER CLASSES (SINCE STREAMGOBBLERS ARE THREAD SPECIFIC)
+
+public class SafeProcess
+    public static int DEBUG = 1;
+
+    public static final int STDERR = 0;
+    public static final int STDOUT = 1;
+    public static final int STDIN = 2;
+    public static String WIN_KILL_CMD;
+    public Boolean interruptible = Boolean.TRUE; 
+
+
+ public SafeProcess(String[] cmd_args)
+ public SafeProcess(String cmdStr)
+ public SafeProcess(String[] cmd_args, String[] envparams, File launchDir)
+  
+
+    public String getStdOutput() { return outputStr; }
+    public String getStdError() { return errorStr; }
+    public int getExitValue() { return exitValue; }
+
+ 
+    public void setInputString(String sendStr) 
+    public void setExceptionHandler(ExceptionHandler exception_handler)
+    public void setMainHandler(MainProcessHandler handler)
+
+    
+    public void setSplitStdOutputNewLines(boolean split)
+    public void setSplitStdErrorNewLines(boolean split)
+
+    public boolean cancelRunningProcess() // calls cancelRunningProcess(!forceWaitUntilInterruptible);
+    public synchronized boolean cancelRunningProcess(boolean forceWaitUntilInterruptible)
+
+
+    public synchronized boolean processRunning()
+
+    public int runBasicProcess()
+    public int runProcess()
+    public int runProcess(CustomProcessHandler procInHandler,
+			   CustomProcessHandler procOutHandler,
+			   CustomProcessHandler procErrHandler)
+    public int runProcess(LineByLineHandler outLineByLineHandler, LineByLineHandler errLineByLineHandler)
+
+
+
+// KEEP AS IS:
+    public static long getProcessID(Process p)
+    static void killWinProcessWithID(long processID)
+    static boolean killUnixProcessWithID(long processID, boolean force, boolean killEntireTree)
+    public static void destroyProcess(Process p)
+
+    // LOGGING - KEEP 
+    public static void log(String msg)
+    public static void log(String msg, Exception e)
+    public static void log(Exception e)
+    public static void log(String msg, Exception e, boolean printStackTrace)
+    public static String streamToString(int src)
+
+	// UTILITY FUNCTIONS - MOVE OUT OF CLASS INTO PACKAGE WHEN WE HAVE SAFEPROCESS AND SAFEPROCESS2
+public static boolean closeResource(Closeable resourceHandle)
+public static boolean closeSocket(Socket resourceHandle)
+public static boolean closeProcess(Process prcs)
+static public boolean processRunning(Process process)
+
+// Uses SafeProcess, but once that is working the new way can KEEP AS IS:
+    public static boolean isAvailable(String program)
+    
+
+INTERNAL INTERFACES AND ABSTRACT CLASSES:
+
+public static interface ExceptionHandler
+    public void gotException(Exception e)
+
+public static interface MainProcessHandler
+    public boolean beforeWaitingForStreamsToEnd(boolean forciblyTerminating);
+    public boolean afterStreamsEnded(boolean forciblyTerminating);
+    public void beforeProcessDestroy(); 
+    public void afterProcessDestroy();
+    public void doneCleanup(boolean wasForciblyTerminated);
+
+public static abstract class CustomProcessHandler
+    public String getThreadNamePrefix()
+    public abstract void run(Closeable stream); //InputStream or OutputStream
+
+
+public static abstract class LineByLineHandler
+    public String getThreadNamePrefix()
+    public abstract void gotLine(String line); // first non-null line
+    public abstract void gotException(Exception e); // for when an exception occurs instead of getting a line
+
+
