How to close std streams from java.lang.Process?

This question is about java.lang.Process and its handling of stdin, stdout and stderr.

We have a class in our project, which is an extension to org.apache.commons.io.IOUtils . There we have a quiet new method for closing std threads of a Process-Object? Or does it not fit?

 /** * Method closes all underlying streams from the given Process object. * If Exit-Code is not equal to 0 then Process will be destroyed after * closing the streams. * * It is guaranteed that everything possible is done to release resources * even when Throwables are thrown in between. * * In case of occurances of multiple Throwables then the first occured * Throwable will be thrown as Error, RuntimeException or (masked) IOException. * * The method is null-safe. */ public static void close(@Nullable Process process) throws IOException { if(process == null) { return; } Throwable t = null; try { close(process.getOutputStream()); } catch(Throwable e) { t = e; } try{ close(process.getInputStream()); } catch(Throwable e) { t = (t == null) ? e : t; } try{ close(process.getErrorStream()); } catch (Throwable e) { t = (t == null) ? e : t; } try{ try { if(process.waitFor() != 0){ process.destroy(); } } catch(InterruptedException e) { t = (t == null) ? e : t; process.destroy(); } } catch (Throwable e) { t = (t == null) ? e : t; } if(t != null) { if(t instanceof Error) { throw (Error) t; } if(t instanceof RuntimeException) { throw (RuntimeException) t; } throw t instanceof IOException ? (IOException) t : new IOException(t); } } 

 public static void closeQuietly(@Nullable Logger log, @Nullable Process process) { try { close(process); } catch (Exception e) { //log if Logger provided, otherwise discard logError(log, "Fehler beim Schließen des Process-Objekts (inkl. underlying streams)!", e); } } 

 public static void close(@Nullable Closeable closeable) throws IOException { if(closeable != null) { closeable.close(); } } 

Methods like these are mostly used in finally blocks.

What do I really want to know if I'm safe with this implementation? Given things like: does a process object always return the same streams stdin, stdout and stderr throughout its life? Or can I skip closing threads previously returned by getInputStream() , getOutputStream() and getErrorStream() ?

Have a question related to StackOverflow.com: java: closing std thread subprocesses?

Edit

As indicated by me and others here:

  • InputStreams must be completely destroyed. When this is not done, the subprocess may not end because there is outstanding data in its output streams.
  • All three std streams must be closed. Regardless of whether it is used before or not.
  • When the subprocess finishes normally, everything should be fine. If not, then this should be forcibly terminated.
  • When the return code is returned by the subprocess, we do not need to destroy() it. He has stopped. (Even if it does not necessarily end normally with exit code 0, but it ends.)
  • We need to keep track of waitFor() and abort when timeout causes the process to finish normally, but kills it when it freezes.

Unanswered parts:

  • Consider the advantages and disadvantages of using InputStream in parallel. Or should they be consumed in a specific order?
+4
source share
3 answers

Just so you know what I have in our code base:

 public static void close(@Nullable Process process) throws IOException { if (process == null) { return; } Throwable t = null; try { flushQuietly(process.getOutputStream()); } catch (Throwable e) { t = mostImportantThrowable(t, e); } try { close(process.getOutputStream()); } catch (Throwable e) { t = mostImportantThrowable(t, e); } try { skipAllQuietly(null, TIMEOUT, process.getInputStream()); } catch (Throwable e) { t = mostImportantThrowable(t, e); } try { close(process.getInputStream()); } catch (Throwable e) { t = mostImportantThrowable(t, e); } try { skipAllQuietly(null, TIMEOUT, process.getErrorStream()); } catch (Throwable e) { t = mostImportantThrowable(t, e); } try { close(process.getErrorStream()); } catch (Throwable e) { t = mostImportantThrowable(t, e); } try { try { Thread monitor = ThreadMonitor.start(TIMEOUT); process.waitFor(); ThreadMonitor.stop(monitor); } catch (InterruptedException e) { t = mostImportantThrowable(t, e); process.destroy(); } } catch (Throwable e) { t = mostImportantThrowable(t, e); } if (t != null) { if (t instanceof Error) { throw (Error) t; } if (t instanceof RuntimeException) { throw (RuntimeException) t; } throw t instanceof IOException ? (IOException) t : new IOException(t); } } 

skipAllQuietly(...) consumes full InputStreams. It uses an internal implementation similar to org.apache.commons.io.ThreadMonitor to interrupt consumption if the specified timeout is exceeded.

mostImportantThrowable(...) decides to return Throwable. Mistakes in everything. First, a higher value occurred than later. There is nothing important here, since these Throwable are likely to be dropped anyway later. We want to continue to work here, and we can only throw it away, so we must decide what we throw in the end, if ever.

close(...) are null implementations to close stuff, but throwing an exception when something goes wrong.

+1
source

Trying to simplify the code:

 public static void close(@Nullable Process process) throws IOException { if(process == null) { return; } try { close(process.getOutputStream()); close(process.getInputStream()); close(process.getErrorStream()); if(process.waitFor() != 0) { process.destroy(); } } catch(InterruptedException e) { process.destroy(); } catch (RuntimeException e) { throw (e instanceof IOException) ? e : new IOException(e); } } 

Throwable I assume that you want to catch all unchecked exceptions. This is either a derivative of RuntimeException or Error . However, Error should never be hooked, so I replaced Throwable with a RuntimeException .

(It is still not recommended to catch all RuntimeException s.)

+2
source

As a question that you associate with states, it is better to read and discard output and error streams. If you are using apache commons io, something like

 new Thread(new Runnable() {public void run() {IOUtils.copy(process.getInputStream(), new NullOutputStream());}}).start(); new Thread(new Runnable() {public void run() {IOUtils.copy(process.getErrorStream(), new NullOutputStream());}}).start(); 

You want to read and discard stdout and stderr in a separate thread to avoid problems such as blocking a process when it writes enough information to stderr or stdout to fill the buffer.

If you are worried about having two threads, see question

I don't think you need to worry about catching IOExceptions when copying stdout, stdin to a NullOutputStream, because if there is a reading of an IOException from the stdout / stdin process, this is probably due to the process itself being dead and writes to NullOutputStream will never throw an exception.

You do not need to check the return status of waitFor ().

Do you want to wait for the process to complete? If so, you can do

 while(true) { try { process.waitFor(); break; } catch(InterruptedException e) { //ignore, spurious interrupted exceptions can occur } } 

After looking at the link you provided, you need to close the threads when the process is complete, but destroy will do it for you.

So, in the end, the method becomes,

 public void close(Process process) { if(process == null) return; new Thread(new Runnable() {public void run() {IOUtils.copy(process.getInputStream(), new NullOutputStream());}}).start(); new Thread(new Runnable() {public void run() {IOUtils.copy(process.getErrorStream(), new NullOutputStream());}}).start(); while(true) { try { process.waitFor(); //this will close stdin, stdout and stderr for the process process.destroy(); break; } catch(InterruptedException e) { //ignore, spurious interrupted exceptions can occur } } } 
+1
source

Source: https://habr.com/ru/post/897619/


All Articles