Exec'ing multiple processes from Java: outputs mixed up?

Question

I have a servlet which creates a new Action object inside doGet(), and this object uses exec() to run external processes. There may be several requests to the servlet at the same time, so I might have several Action objects where each one is running an external process at the same time. Occasionally when this happens I find that the output from one process gets mixed up with the output from one of the others.

Each Action creates a unique temporary directory and runs the process with this as its current directory. Separate threads then read the output streams from the process into a string buffer. The code that the Action object executes looks basically like this:

Process proc = null;
ReaderThread stdout = null;
ReaderThread stderr = null;
StringBuffer buff = new StringBuffer();
int exit = -1;
try {
  //
  //  Run the process
  //
  proc = Runtime.getRuntime().exec(command,null,directory);
  //
  //  Read the output from the process
  //
  stdout = new ReaderThread(proc.getInputStream(),buff);
  stderr = new ReaderThread(proc.getErrorStream(),buff);
  stdout.start();
  stderr.start();
  //
  //  Get the exit code
  //
  exit = proc.waitFor();
  //
  //  Wait for all the output to be read
  //
  stdout.join();
  stderr.join();
}
catch (InterruptedException e) {
  if (proc != null) {
    proc.destroy();
  }
}
catch (Exception e) {
  buff.append(e.getClass() + ": " + e.getMessage());
  if (proc != null) {
    proc.destroy();
  }
}

So each request uses a separate Action object to run a process, and this has its own StringBuffer "buff" that the output of the process is accumulated into by the two ReaderThreads. But what I find is that, when two requests are running two processes at the same time, the output of one will sometimes end up in the StringBuffer of the thread that is running the other one, and one of the two servlet requests will see output intended for the other one. It basically behaves as if Runtime.exec() provides a single global pipe to which the output streams of all the processes are connected.

The ReaderThread looks like this:

public class ReaderThread extends Thread {
  private BufferedReader reader;
  private StringBuffer buffer;

  public ReaderThread (InputStream stream, StringBuffer buffer) {
    this.reader = new BufferedReader(new InputStreamReader(stream));
    this.buffer = buffer;
  }

  @Override
  public void run () {
    try {
      String line;
      while ((line = reader.readLine()) != null) {
        synchronized (buffer) {
          buffer.append(line + "\n");
        }
      }
    }
    catch (IOException e) {
      synchronized (buffer) {
        buffer.append(e.getMessage() + "\n");
      }
    }
  }
}

Can anyone suggest what I can do to fix this?

please show full Servlet and Action code. i think the mistake lies there, because these snippets are correct. — Michele Mariotti, Dec 31 '13 at 14:10
If you use ProcessBuilder to execute your command, you can make use of the [`ProcessBuilder.redirectErrorStream`](http://docs.oracle.com/javase/7/docs/api/java/lang/ProcessBuilder.html#redirectErrorStream%28boolean%29) method, which will allow you to simply your own code by having only one ReaderThread per process, plus you can remove all the synchronization from your ReaderThread class. Simplifying the code should make the problem easier to track down. — VGR, Dec 31 '13 at 14:43
Kind of hard, unless you want me to post about 35,000 lines of code! Trust me, the servlet creates a brand-new Action in doGet(), the Action reads a pile of XML from a database, and one of the XML nodes creates the process as above. If these snippets are correct, and I can't see anything wrong with them either, the only other explanation I can think of is a bug in Runtime.exec(). — user1636349, Dec 31 '13 at 14:48
I've managed to make a reproducible test case (ProcessBuilder did make life easier here, BTW), and it appears that the error is indeed somewhere else. I'm now on the track of it. Apologies. — user1636349, Dec 31 '13 at 17:24
Here's the explanation, partly as a cautionary tale: the output was being added to an ArrayList in the XML node that accessed it. The XML nodes were created by cloning prototypes. The ArrayList was initialised in the declaration, not in the initialise() method of the class, so every instance ended up referring to the same object. Duh. Another two days of my life down the drain! — user1636349, Dec 31 '13 at 17:42

score 0 · Answer 1 · edited May 23 '17 at 12:28

0

Use a ThreadLocal variable to store the output of each thread.

When and how should I use a ThreadLocal variable?

edited May 23 '17 at 12:28

Community

1
1

answered Dec 31 '13 at 13:47

Andres

10,561
4
45
63

ThreadLocal is supposed to make shared variables thread-safe. But in my code there are no variables shared between the threads. The problem is actually the complete opposite: output that should be put in one thread's StringBuffer appear in a completely different StringBuffer in another thread. The only explanation I can think of is that both reader threads are reading from the same output pipe. – user1636349 Dec 31 '13 at 14:40
If that is the case, I'd try to read the processes output in an asynchronous way. – Andres Dec 31 '13 at 15:44
I'd "desynchronize" more the read and write processes. Why don't you read the full output after the write process finishes? – Andres Dec 31 '13 at 18:58
Because then the normal and error output wouldn't be interleaved properly. It's easier now I'm using ProcessBuilder though... – user1636349 Dec 31 '13 at 21:02

score 0 · Accepted Answer · answered Jan 01 '14 at 10:45

Here's the explanation, partly as a cautionary tale:

the output was being added to an ArrayList in the XML node that accessed it
the XML nodes were created by cloning prototypes
the ArrayList was initialised in the declaration, not in the initialise() method of the class, so every instance ended up referring to the same object.

Duh.

Another two days of my life down the drain!

Happy new year...

Exec'ing multiple processes from Java: outputs mixed up?

2 Answers2