External process calls from java: Runtime.exec() hangs

My recent blog was about calling wget from Java which was talking about a temporal solution. The solution was just to mute the output of the external process so that the output buffer never gets exhausted. Unfortunately, it doesn't solve the bigger problem of calling external processes from Java through Runtime.exec() which can very easily exhaust the system buffer and cause a deadlock at Process.waitFor().

I read a hell lot of articles and spent a couple of hours, looking for the solutions. I got many but all of them were not solving my problem. Then I checked one of the solutions and discussed it at #fedora-java at freenode to see if the problem is really with Java in Fedora! With the help of mizdebsk I was able to spot the concurrency issue in the code which I have posted here. The conclusion after the incorporation of the following code was the external process took some abnormally more time at some point in the program but in the end it was successful to finish rather than waiting forever. I have just defined the block to handle output here, similar block for ErrorStream should be included and if your process expects some input then it should be added too.

class ExecCommand {
private Semaphore outputSem;
private String output;
private Semaphore errorSem;
private String error;
private Process p;

private class OutputReader extends Thread {
 public OutputReader() {
  try {
  outputSem = new Semaphore(1);
  outputSem.acquireUninterruptibly();
  } 
  finally {
   outputSem.release();
  }
 }

 public void run() {
  try {
  StringBuffer readBuffer = new StringBuffer();
  BufferedReader isr = new BufferedReader(new InputStreamReader(p
   .getInputStream()));
  String buff = new String();
  while ((buff = isr.readLine()) != null) {
   readBuffer.append(buff);
   System.out.println(buff);
  }
  output = readBuffer.toString();
  outputSem.release();
  } catch (IOException e) {
  e.printStackTrace();
  }
 }
}

public ExecCommand(String command) {
 try {
  p = Runtime.getRuntime().exec(makeArray(command));
  new OutputReader().start();
  new ErrorReader().start();
  p.waitFor();
 } catch (IOException e) {
  e.printStackTrace();
 } catch (InterruptedException e) {
  e.printStackTrace();
 }
}

public String getOutput() {
 try {
   outputSem.acquireUninterruptibly();
 } 
 finally {
   outputSem.release();
 }
 String value = output;
 outputSem.release();
 return value;
}

private String[] makeArray(String command) {
 ArrayList<String> commandArray = new ArrayList<String>();
 String buff = "";
 boolean lookForEnd = false;
 for (int i = 0; i < command.length(); i++) {
  if (lookForEnd) {
  if (command.charAt(i) == '\"') {
   if (buff.length() > 0)
   commandArray.add(buff);
   buff = "";
   lookForEnd = false;
  } else {
   buff += command.charAt(i);
  }
  } else {
  if (command.charAt(i) == '\"') {
   lookForEnd = true;
  } else if (command.charAt(i) == ' ') {
   if (buff.length() > 0)
   commandArray.add(buff);
   buff = "";
  } else {
   buff += command.charAt(i);
  }
  }
 }
 if (buff.length() > 0)
  commandArray.add(buff);

 String[] array = new String[commandArray.size()];
 for (int i = 0; i < commandArray.size(); i++) {
  array[i] = commandArray.get(i);
 }

 return array;
}
}

Add this class to your code and use the following code to call it for you.

String command = "your external command command";
         
ExecCommand e;
e = new ExecCommand(command);

This takes care of almost all the things, parsing the command line and printing the output to the console. Moreover, sometimes I have noticed that adding a sleeptime in the ExecCommand constructor also helps.

p = Runtime.getRuntime().exec(makeArray(command));
  new OutputReader().start();
  new ErrorReader().start();
  int sleeptime = 1000; // Try to make an educated guess 
                        // about the sleeptime
  Thread.sleep(sleeptime);
  p.destroy();

This block of code is the corrected version the code given <Here>. Basically the changes are related to outputSem.acquire() to outputSem.acquireUninterruptibly() and adding the finally block. Without them it was easily leading to a deadlock.