David Walend's Blog

Connecting to a Command Line Process

Part of this project was adding a tographviz package to JDigraph to encapsulate interaction with the dot language and the Graphviz command line. I built some fairly elaborate code to wrap around the Graphviz command line. I created the GraphvizControl.java class to bundle it all up. It makes for a nice blog article.

I discovered early on that the operating system offered me a fairly short time to read in stderr and stdout. Sometimes I would get all of the output from dot, sometimes just pieces of an error message, or part of the drawing. I solved this problem by creating the StreamPull inner class:

/**
Reads an InputStream until the stream reaches its end. 
The contents of the stream are available in getResult(). 
Any exceptions encountered are in getTrouble().
*/
    private static class StreamPull
        implements Runnable
        {
            private InputStream is;
            private final Object guard = new Object();
            private String result = null;
            private IOException trouble = null;
            
            StreamPull(InputStream is)
            {
                this.is = is;
            }
            
            public void run()
            {
                try
                {
                    StringBuilder stringBuilder = new StringBuilder();
                
                    InputStreamReader streamReader = new InputStreamReader(is);
                    
                    int i;
                    while((i = is.read()) != -1)
                    {
                        char c = (char)i;
                        stringBuilder.append(c);
                    }
                    synchronized(guard)
                    {
                        result = stringBuilder.toString();
                    }
                }
                catch(IOException ioe)
                {
                    synchronized(guard)
                    {
                        trouble = ioe;
                    }
                }
                finally
                {
                    try
                    {
                        is.close();
                    }
                    catch(IOException ioe)
                    {
                        synchronized(guard)
                        {
                            if(trouble==null)
                            {
                                trouble = ioe;
                            }
                        }
                    }
                }
            }
            
            String getResult()
            {
                synchronized(guard)
                {
                    return result;
                }
            }
            
            IOException getTrouble()
            {
                synchronized(guard)
                {
                    return trouble;
                }
            }
        }

Is there a better way to read an InputStream to its end? while((i = is.read()) != -1) looks like 1985. Shouldn't we have left that behind with the piano keyboard tie?

The other ugly part is that massive finally{} block. Four }s reminds me of lisp, but at least the error code is not mixed in to the rest.

The makePicture() method ties together the Process, the InputStreams, StreamPulls, Threads, and an OutputStream. It gets the process out of a ProcessBuilder, grabs the stdout and stderr streams from the process, feeds them to StreamPulls, starts Threads for the StreamPulls, and then writes the instructions to the process. After that, it waits for the process to complete, then joins the two StreamPulls. I didn't use buffers on the streams; the code seems to get by without them just fine. (Your profiler may say something different. Let me know if it does.) I wish Process had a waitFor() method with a time out so that I could add some warning against infinite loops short of calling process.destroy(). However, dot is a pretty solid program with some years of use behind it.

    public String makePicture(String dotString,String process,String format)
        throws GraphvizControlException
    {
        try
        {
            Process dotProcess = new ProcessBuilder(process, "-T"+format).start();
    
            InputStream fromDotStream = dotProcess.getInputStream();
            InputStream errorDotStream = dotProcess.getErrorStream();
    
            StreamPull errorDotPull = new StreamPull(errorDotStream);
            StreamPull fromDotPull = new StreamPull(fromDotStream);
            
            Thread errorThread = new Thread(errorDotPull);
            Thread fromThread = new Thread(fromDotPull);
            
            errorThread.start();
            fromThread.start();
            
            //write to standard out for the process
            OutputStream toDotStream = dotProcess.getOutputStream();
            PrintWriter printWriter = new PrintWriter(new OutputStreamWriter(toDotStream));
            
            printWriter.write(dotString);
            printWriter.close();
    
            dotProcess.waitFor();
            
            errorThread.join(1000);
            fromThread.join(1000);
            

And then there's this massive block of error handling code that opens with a colossal if statement. It translates to "if anything went wrong, build up an error message and throw an Exception." After that code there's one line that returns dot's output as a String, and a little more exception handling.

            if(errorDotPull.getTrouble()!=null 
                || ((errorDotPull.getResult()!=null)&&(!errorDotPull.getResult().equals(""))) 
                || fromDotPull.getTrouble()!=null 
                || fromDotPull.getResult()==null)
            {
                StringBuffer buffer = new StringBuffer();
                IOException cause = null;
                if((errorDotPull.getResult()!=null)&&(!errorDotPull.getResult().equals(""))) 
                {
                    buffer.append(errorDotPull.getResult());
                }
                if(fromDotPull.getResult()==null) 
                {
                    buffer.append("result from process is null");
                }
                if(fromDotPull.getTrouble()!=null)
                {
                    buffer.append("IOException on external process' output stream.");
                    cause = fromDotPull.getTrouble();
                }
                if(errorDotPull.getTrouble()!=null)
                {
                    buffer.append("IOException on external process' error stream.");
                    cause = errorDotPull.getTrouble();
                }
                if(cause == null)
                {
                    throw new GraphvizControlException(buffer.toString());
                }
                else
                {
                    throw new GraphvizControlException(buffer.toString(),cause);
                }
            }
            
            return fromDotPull.getResult();
        }
        catch(IOException ioe)
        {
            throw new GraphvizControlException(ioe);
        }
        catch(InterruptedException ie)
        {
            throw new GraphvizControlException(ie);
        }
    }

I'm mostly happy with the way this class came out, but the whole works has the feel of something that should have just been there. I normally like the library Sun provides in Java, but I felt like I was working far from the context of my problem while bringing this class together. Building the code felt a little strange, like I was assembling a box of many small parts that should have come in larger, pre-assembled chunks. Creating Swing code often feels the same way to me, but the io package always feels fine. I think it's familiarity vs. complexity.

Comments

• InputStream.read() returns the next byte, so casting it to a char only works for a limited character set in some encodings. If you're reading binary it's better to read into a byte array rather than individual bytes, though as it seems it's and ASCII text file, using BufferedReader and readLine() may be better.

  I'd add a name field to StreamPull, and either have getResult() throw trouble if not null or it thrown by a reportExceptions() method, rather than all the error handling in the client. You'd want to subclass StreamPull for the error stream so its reportExceptions() method throws an exception whenever the data read is not empty; I tend to prefer specialisation by Oo rather than by procedural conditions.

  Posted by: pete_kirkham on August 25, 2005 at 05:42 AM

• imho, some things to consider are:
  
  (1) Write an abstraction for a process whose stdout and stderr are assumed to be bounded by some constant byte size passed in to the constructor
  (2) Make StreamPull an inner class of that
  (3) Maybe use a BufferedReader and read a line of output at a time?
  (4) there seem to be an excessive number of combinations of error possibilities considered. In practice surely you want to check the return code of the process, and if its success assume the stdout is a good form. I'm not sure what value there is in differentiating between a null stdout and the empty string stdout for example?
  (5) maybe use ProcessBuilder to redirect stderr to stdout if the client code doesn't need to distinguish the two?
  
  
  jakarta commons io has some utility methods for sucking up the content of readers and inputstreams in their entirety. There doesn't seem good support for this in the JDK? It would've been nice to have an interface for InputStream and a subclass FiniteInputStream or BoundedInputStream or something which included methods for getting the whole inputstream or reader output as an array or String :)

  Posted by: asjf on August 25, 2005 at 08:37 AM

• Something else you might be interested in is Expectj, a Java port of Expect. It allows you to automate the interaction with other processes via the command line.

  Expect is a very powerful automation utility that's been known and used for ages in the UNIX world. See http://expect.nist.gov. Activestate has a Windows port.

  I haven't used the Java version yet, but it's in my bag-of-tools for future use. You might gleam something useful from the sources, or just use it within your app. See http://expectj.sourceforge.net

  Posted by: mgbacke on August 25, 2005 at 09:54 AM

• I'd replace the code in StreamPull.run by using org.apache.commons.io.IOUtils: public void run() { try { result = IOUtils.toString(is); } catch (Exception e) { setException(e); } finally { IOUtils.close(is); } } I also think that you might want to use java.util.concurrent.FutureTask to implement the StreamPull, but it's been several years since I used the concurrent library, so I don't remember how. :-( Cheers, ~Johannes

  Posted by: jhannes on August 25, 2005 at 10:14 AM

• Thanks for the suggestions. I'll try them out next week and report in.

  Posted by: dwalend on August 26, 2005 at 10:37 AM

• pete_kirkham -- I switched from a StringBuilder to ByteArrayOutputStream's toString() method. That should fix any unicode issues. I don't much like the idea of extending StreamPull by how something larger will interpret it; something on sys.err isn't an Exception until the context says so. asjf -- I don't get the luxury of a known byte array size, but ByteArrayOutputStream does a nice job. Thanks for reminding me to check error codes. I haven't spotted information on what dot might return, but I think I know where to ask. mgbacke -- Thanks for the pointer to expectj. It looks way over the top for what I need. I don't want to add another programming language to the mix; java, dot, and command line are plenty. jhannes -- I'll keep FutureTask in mind in case the dot processes start to run long.

  Posted by: dwalend on September 05, 2005 at 05:07 PM

This work is licensed under a Creative Commons License.