Hanging forever when executing a shell command (Java)

Question

The offending block of code is below. The code almost always works, but sometimes it hangs forever. The application is an EJB timer bean.

Actually, it has only hung once and I can't reproduce it. It's worked without any problems in production for almost 2 years. But, while testing an updated version of the app, the timer just froze after running for a few days and never released the database locks from it's last run. The logs clearly show that it froze somewhere in the block of code below. The command it's running is 'chmod'.

public void shellExec(String cmd, File workDir) {
    String s = null;
    try {
        Process p = Runtime.getRuntime().exec(cmd, null, workDir);
        int i = p.waitFor();
        if (i == 0){
            BufferedReader stdInput = new BufferedReader(new InputStreamReader(p.getInputStream()));
            // read the output from the command
            while ((s = stdInput.readLine()) != null) {
                logger.debug(s);
            }
        }
        else {
            BufferedReader stdErr = new BufferedReader(new InputStreamReader(p.getErrorStream()));
            // read the output from the command
            while ((s = stdErr.readLine()) != null) {
                logger.debug(s);
            }
        }
    } catch (Exception e) {
        logger.debug(e);
    }
}

I'm hesitant to modify this code because it's been tested and has worked correctly for almost two years. I also can't reproduce the problem so I'd have no idea whether a rewritten version is any better. But, it's pretty clear that it could hang and I don't know what the likelihood is.

From googling the issue, it seems like that block of code is pretty standard for executing a shell command. Is there any kind of known problem with that code? Does anyone know of a good way to make sure that it will throw an exception rather than hang, considering that I can't reproduce the problem?

Thanks.

Answer 1

You need to execute the consumption of stdout/err concurrently. Otherwise you'll get the blocking behaviour that you see. See this answer for more info.

Answer 2

Related to the consuming of stderr and stdout, here's a little convenience class which I use frequently:

import java.io.InputStream;
import java.io.OutputStream;

public final class Pipe implements Runnable {

    private final InputStream in;
    private final OutputStream out;

    public Pipe(InputStream in, OutputStream out) {
        this.in = in;
        this.out = out;
    }

    public static void pipe(Process process) {
        pipe(process.getInputStream(), System.out);
        pipe(process.getErrorStream(), System.err);
        pipe(System.in, process.getOutputStream());
    }

    public static void pipe(InputStream in, OutputStream out) {
        final Thread thread = new Thread(new Pipe(in, out));
        thread.setDaemon(true);
        thread.start();
    }

    public void run() {
        try {
            int i = -1;

            byte[] buf = new byte[1024];

            while ((i = in.read(buf)) != -1) {
                out.write(buf, 0, i);
            }
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

Super simple and does the trick without any extra libraries.