Should BufferedReader and InputStreamReader be closed explicitly?

Question

I want to read the content of a InputStream into a String:

private String readToString(InputStream stream) {
    return new BufferedReader(new InputStreamReader(stream))
            .lines().collect(Collectors.joining("\n"));
}

The stream comes from java.lang.Process.

Question: Do I have to explicitly close any of the InputStream, InputStreamReader or BufferedReader in this case?

Sidenote: the linked question is NOT a duplicate, as my question is about HOW to properly close the streams, not how to read the stream to a String!

The [javadoc for `lines`](https://docs.oracle.com/javase/8/docs/api/java/io/BufferedReader.html#lines--) seems to imply that the stream doesn't get closed automatically. — assylias, Nov 17 '17 at 09:47
just a side not you are getting the `InputStream` from a `File` most probably. why not use `File.lines` directly? — Eugene, Nov 17 '17 at 09:49
Do not close anything. If you close the BufferedReader or the InputStreamReader, they will implicitly also close the InputStream. But that responsibility should lie with the one who opened it (preferably inside a try-with-resouces). Furthermore, if you want to read the whole text into a String, don't read line-by-line only to rejoin the lines together into a single string. There's nothing special about the newline character that should mean the file has to be split like that. Instead, read the whole file as bytes and convert them into chars using your chosen encoding. — DodgyCodeException, Nov 17 '17 at 09:54
Possible duplicate of [Read/convert an InputStream to a String](https://stackoverflow.com/questions/309424/read-convert-an-inputstream-to-a-string) — Klitos Kyriacou, Nov 17 '17 at 09:58
@KlitosKyriacou that question is from 2008, almost 10 years have passed — Eugene, Nov 17 '17 at 10:00
@Eugene it's still valid though. Just because new things have been introduced doesn't mean they're better than existing ones. Besides, the OP just wants to read the file into a single string, so there's no need to split it into lines first, and Scanner with suitable delimiter will do the job nicely. — DodgyCodeException, Nov 17 '17 at 10:02
@DodgyCodeException in fact, some of the answers there are quite recent even if the question is old! One of the answers uses a Java 9 feature. — Klitos Kyriacou, Nov 17 '17 at 10:22
I've just seen your edit and take your point about this issue not being a duplicate. I've retracted my close vote. — Klitos Kyriacou, Nov 17 '17 at 11:14

score 6 · Accepted Answer · answered Nov 17 '17 at 09:53

You only need to close the outer wrapper, but don't do that explicitly either way - there is try-with-resource that will make your life easier:

public String readToString(InputStream stream) {

    try (InputStreamReader reader = new InputStreamReader(stream);
            BufferedReader br = new BufferedReader(reader)) {

        return br.lines().collect(Collectors.joining("\n"));

    } catch (IOException e) {
        e.printStackTrace();
        throw new RuntimeException(e);
    }
}

There is also a much easier way to do that and way more clear:

Files.readAllLines(YourPath)

I'm getting the inputstream from `java.lang.process so cannot use `Files`. But try-with is ok. — membersound, Nov 17 '17 at 09:58

DodgyCodeException · Answer 2 · 2017-11-17T11:18:51.333

As per my comment, closing either the BufferedReader or the InputStreamReader will cause the InputStream to be closed. Your readToString method should not close the stream. This is the responsibility of the caller.

The rationale:-

Firstly, consider how you open the stream before you call readToString. A sensible way to do it is:

try (InputStream myStream = getInputStreamSomehow()) {
    //...
    String content = readToString(myStream);
    //...
}

The stream will be closed at the end of your try-with-resources block.

Secondly, consider existing best practices and idioms. Look at Java API methods that, just like your method, read the whole contents of a stream. For example, from Java 9:

Neither of the above methods closes the stream. Similarly, the users of your readToString method will not be expecting you to close their stream.

Should BufferedReader and InputStreamReader be closed explicitly?

2 Answers2

Linked