Closing BufferedReader and InputStreamReader

Question

This piece of code is creating memory leak issues cause of BufferedReader and InputStreamReader which I think might be happening cause of some exceptions. How should I change it?

try{
    URL url = new URL(sMyUrl);
    BufferedReader in = new BufferedReader(new InputStreamReader(url.openStream()));
    while ((str = in.readLine()) != null) {
        jsonString += str;
    }
    in.close();
}catch(Exception e){

}

Brune

May be move close() logic to finally block? Are you sure no exceptions happening while reading (or) closing connection? – Poison

Have tried writing e.printStackTrace() in your catch-clause to see if exceptions where thrown? – Preshrunk

Answer 1

It would be safer to close your stream using a try..finally block. You might also use a StringBuilder as it is designed for concatenating strings. You should also avoid catching Exception and doing nothing with it. Also, your code is concatenating lines without any line-breaks. This may well not be what you want, in which case append("\n") when you read each line in.

Here's a version with those modifications:

StringBuilder json = new StringBuilder();
try {
    URL url = new URL(sMyUrl);
    BufferedReader in = new BufferedReader(new InputStreamReader(url.openStream()));
    try {
        String str;
        while ((str = in.readLine()) != null) {
            json.append(str).append("\n");
        }
    } finally {
        in.close();
    }
} catch (Exception e) {
    throw new RuntimeException("Failed to read JSON from stream", e);
}

Amatruda

Do I need to close InputStreamReader ? Or its closed as Buffer reader will create a new object? – Brune

No need to explicitly close the InputStreamReader as it will be closed automatically when you close the BufferedReader. – Exegete

@aetheria, thank you. Anyhow I need a good Java streams tutorial. – Cavitation

@aetheria I have seen in some tutorials, the resource is being checked for null ness before closing. But when I try that one it asked me to enclosed them in another try catch block. why is that? and is it essential to check for null ness before closing a resource – Wool

Kasun, I suggest you post a separate question with an example. – Exegete

This answer is incorrect as it doesn't even compile (at least in java7). Variable BufferedReader in is declared in scope of try block and is not visible in scope of finally block. The variable may not even get assigned in case MalformedURLException is thrown from URL class constructor. – Athalie

No, this is a misreading. There are two try blocks. The variable in is not defined inside the try block that closes it. – Exegete

@Amatruda You're right. Something must be wrong with me then. Sorry for confusion. – Athalie

Answer 2

The code isn't pretty but won't be creating a memory leak. I suggest you use a memory profiler to determine where your memory is being used. Otherwise you are just guessing even if you have ten + years experience performance tuning in Java ;)

A better alternative is to use Java 7

URL url = new URL(sMyUrl);
try(BufferedReader in = new BufferedReader(new InputStreamReader(url.openStream()))) {
  while ((str = in.readLine()) != null) {
     jsonString.append(str).append("\n");
  }
}

If you have Java 6 or older you can use.

BufferedReader in = new BufferedReader(new InputStreamReader(url.openStream()))) {
try {
  while ((str = in.readLine()) != null) {
     jsonString.append(str).append("\n");
  }
} finally {
  in.close();
}

Lhary

Thanks ! Do I need to close InputStreamReader ? – Brune

@SoneshDabhi When you close the BufferedReader it closes InputStreamreader which closes the InputStream you got from openStream() etc. – Lhary