Do I need to close InputStreamReader even if InputStream should remain open?

Question

The InputStream is passed as a parameter from somewhere, where it will be further processed and then closed. So I don't want to close the InputStream here. Consider the following code:

void readInputStream(final InputStream inputStream) {
    final BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(inputStream));
    String line;
    while ((line = bufferedReader.readLine() != null) {
        // do my thing
    }
}

If I close the BufferedReader and/or the InputStreamReader, then the underlying InputStream will be closed as well, according to another Stackoverflow post.

My question: Do Readers need to be closed, even if the underlying InputStream is closed somewhere else? Can I get a memory leak by not closing Readers?

Craniometry

I don't fully understand your problem but in general yes resources like so should be closed once you're done using them. Also I suggest to have a look at try-with-resources which will auto close them for you. Finally your code snippet is not the most appropriate for this question. – Maddi

Your method shouldn't probably create its own BufferedReader, but accept it as argument. This way you will have one BufferedReader in your application and can close it when it is no longer needed which will also close underlying InputStream. – Astridastride

@Astridastride The InputStream is passed around to different handlers which do different things with it. Each time it is reset before (and closed at the end). Only this handler actually needs a BufferedReader, another handler just passes the InputStream to a library function and so on. – Craniometry

Answer 1

Do readers need to be closed, even if the underlying InputStream is closed somewhere else?

No they absolutely don't need to be in that scenario. But it is generally a good idea to close them anyway.

Can I get a memory leak by not closing readers?

No, there is no memory leak, assuming that the Reader itself becomes unreachable once you have finished with it. And besides, a Reader doesn't typically use a lot of memory.

The more important question is whether you can get a resource leak by not closing a Reader. The answer is ... it depends.

• If you can guarantee that the underlying InputStream will always be closed somewhere else in the application, then that takes care of possible memory leaks.

• If you can't guarantee that, then there is a risk of a resource leak. The underlying OS level file descriptors are a limited resource in (for example) Linux. If a JVM doesn't close them, they can run out and a certain system calls will start to fail unexpectedly.

But if you do close the Reader then the underlying InputStream will be closed.

Calling close() more than once on a InputStream is harmless, and costs almost nothing.

The only case where you shouldn't close the Reader is when it would be wrong to close the underlying InputStream. For example, if you close a SocketInputStream the rest of the application may not be able to reestablish the network connection. Likewise, the InputStream associated with System.in usually cannot be reopened.

In that case, it is actually safe to allow the Reader you create in your method to be garbage collected. Unlike InputStream, a typical Reader class doesn't override Object::finalize() to close its source of data.

---

@Pshemo brought up an important point about system design.

If you are accepting an InputStream as an argument, then it may be wrong to wrap it with a local Reader ... especially a BufferedReader. A BufferedReader is liable to read-ahead on the stream. If the stream is going to be used by the caller after your method returns, then any data that has been read into the buffer but not consumed by this method is liable to be lost.

A better idea would be for the caller to pass a Reader. Alternatively, this method should be documented as taking ownership of the InputStream. And in that case, it should always close() it.

Bridgeport

My use-case is that the user can upload a file and this file can be interpreted by many different handlers. So the first handler tries to read the file in a specific format, and if it's not that format, the next handler will read the file and so on. That inputstream is reset every time a new handler reads it. That's why it is passed around. But your answer confirms what I thought, thank you :) – Craniometry

Answer 2

Yes, Readers need to be closed. Use a proxy, e.g. CloseShieldInputStream, to prevent the passed parameter from being closed.

void readInputStream(InputStream inputStream) throws IOException{

  try (var bufferedReader = new BufferedReader(new InputStreamReader(
       new CloseShieldInputStream(inputStream)))) {

    String line;
    while ((line = bufferedReader.readLine()) != null) {
      // do my thing
    }
  }
}

JIC: Similar to the input shield, Apache Commons I/O also provides an output shield to solve the similar problem with closing a wrapping output stream, - CloseShieldOutputStream

---

For more detailed considerations, please refer to the original answer. Credits to @stephen-c

Claudio