Question

In my Java code, I start a new process, then obtain its input stream to read it:

BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));

FindBugs reports an error here:

may fail to close stream
Pattern id: OS_OPEN_STREAM, type: OS, category: BAD_PRACTICE

Must I close the InputStream of another process? And what's more, according to its Javadoc, InputStream#close() does nothing. So is this a false positive, or should I really close the input stream of the process when I'm done?

Was it helpful?

Solution

In this case, you want to close() the Reader, which will close its underlying streams. Yes, it's always good practice to close streams, even if at the moment you know the implementation you're looking at doesn't do anything (though, in fact, it does here!). What if that changed later?

FindBugs is only there to warn about possible errors; it can't always know for sure.

Finally yes, your Java process owns the process and Process object you spawned. You most definitely need to close that and the output stream. Nobody else is using them, and, it's important to do such things to avoid OS-related stream funny business.

OTHER TIPS

InputStream is an abstract class - just because its implementation does nothing doesn't mean that the actual type of object returned by process.getInputStream() doesn't.

It's possible that failing to close the input stream in this particular case would do no harm - but I personally wouldn't count on it. Close it like you'd close any other input stream. Aside from anything else, that makes your code more robust in case you ever decide to change it to read from something else - it would be all too easy to (say) read from a file instead, and not notice that you're not closing the FileInputStream.

I think its always a good practice to close all the streams you open. Preferably in the finally{} block. Since it does nothing as java says, why not call the close() method. Its of no harm.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top