Shell command not executed

Question

I have a weird problem when trying to execute a shell command from within a java program. Since there exist thousands of websites that explain how to do it I used the following recommended code:

public String executeShellCommand (String command)
{
    try
    {
        StringBuffer sb = new StringBuffer();
        String line = "";

        Process p = Runtime.getRuntime().exec(command);
        BufferedReader reader = new BufferedReader(new InputStreamReader(
                p.getInputStream()));
        while ((line = reader.readLine()) != null)
            sb.append(line + "\n");
        p.waitFor();

        return sb.toString();
    }
    catch (Exception e)
    {
        e.printStackTrace();
    }
    return null;
}

Acutally, when I try to execute for instance ls -aF is works fine and I get some output as a result. Therefore I'm pretty sure that the above code is, in principal, correct. However, I got another program I'd like to run and that produces a file as an output. I would like to execute it the above way but it never is executed and no output file is generated. Also I do not get any error, warnings or whatsoever in java. When copy and pasting the actual command argument string into the console the execution of the programm/command directly in the shell works fine and the output file is generated. So the command I pass to the method is also correct.

Are there additional things I need to pay attention to when trying to execute a shell command from within java?

UPDATE: I modified my code according to the suggestions. However, it is still hanging:

    public String executeShellCommand(List<String> command, String logfile, boolean waitForProcess) { try {
    ProcessBuilder pb = new ProcessBuilder(command);
    System.out.println("pb.toString() = " + pb.toString());
    Process p = pb.start();
    System.out.println("2");
    BufferedReader err = new BufferedReader(new InputStreamReader(p.getErrorStream()));
    BufferedReader out = new BufferedReader(new InputStreamReader(p.getInputStream()));
    System.out.println("3");
    StringBuilder errSb = new StringBuilder();
    StringBuilder outSb = new StringBuilder();
    String line;
    System.out.println("4");
    while ((line = err.readLine()) != null) { // <--- code hangs here
        errSb.append(line + "\n");
        System.out.println("errSb = " + errSb.toString());
    }
    System.out.println("4a");
    while ((line = out.readLine()) != null) { 
        outSb.append(line + "\n");
        System.out.println("outSb = " + outSb.toString());
    }

    System.out.println("5");

    if(waitForProcess) {
        System.out.println("Wait for process");
        p.waitFor();
    } else {
        System.out.println("Sleep 5000");
        Thread.sleep(5000);
    }
    System.out.println("6");
    //Log result to file
    if(logfile != null) {
        OutputStreamWriter outWriter = new OutputStreamWriter(new FileOutputStream(logfile));
        outWriter.write(errSb.toString());
        outWriter.close();
    }
    return errSb.toString();
} catch(Exception e) { e.printStackTrace(); } return null; }

Answer 1

This will block if your command writes too many characters to stderr. Like for sdtout, Java redirect stderr through a pipe, and if you do not read the pipe, it can fill up and block (size of the pipe is probably less than 256 bytes). To avoid that, you need to read from the Process.getErrorStream(), preferable from another thread as the main thread is busy reading from the Process.getInputStream().

A simpler way to avoid that is to use the ProcessBuilder class instead of Runtime.exec() and ProcessBuilder.redirectErrorStream(true) so that both stdout and stderr are merged into the Process.getInputStream()

Answer 2

As per Process javadoc :

Because some native platforms only provide limited buffer size for standard input and output streams, failure to promptly write the input stream or read the output stream of the subprocess may cause the subprocess to block, or even deadlock.

You are calling p.waitFor(). If we carefully read the waitFor() documentation:

Causes the current thread to wait, if necessary, until the process represented by this Process object has terminated.

You are waiting for a process which hangs, because its error stream and output stream are never read.

What you should do, is to read these streams:

p.start();

BufferedReader err= new BufferedReader(new InputStreamReader(p.getErrorStream()));
BufferedReader out = new BufferedReader(new InputStreamReader(p.getOutputStream()));

StringBuilder errSb = new StringBuilder();
StringBuilder outSb = new Stringbuilder();

String line;
while ((line = err.readLine()) != null) {
    errSb.append(line);
}

while ((line = out.readLine()) != null) {
    outSB.append(line);
}

int retCode = p.waitFor(); //0 for success
System.out.println(retCode);
System.err.println(errSB.toString());

You should always read the error stream when calling external programs via the Process class, else you may find yourself in this odd situation where a process hangs forever. (well until someone else -the operating system, another application, etc- kills it, more exactly).

---

I've also noticed that you use the Runtime.getRuntime() which is not the recommended way to run external programs, starting with java 1.5, as per javadoc:

As of 1.5, ProcessBuilder.start() is the preferred way to create a Process.

ProcessBuilder pb = new ProcessBuilder("ls" , "-aF");
Process p = pb.start();