Closing BufferedReader and InputStreamReader

42,442

Solution 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);
}

Solution 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();
}

42,442

Author by

Pit Digger

Updated on March 24, 2020

Comments

Pit Digger about 4 years

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){

}

Pit Digger over 11 years

Thanks ! Do I need to close InputStreamReader ?
Pit Digger over 11 years

Do I need to close InputStreamReader ? Or its closed as Buffer reader will create a new object?
ᴇʟᴇvᴀтᴇ over 11 years

No need to explicitly close the InputStreamReader as it will be closed automatically when you close the BufferedReader.
Vishy over 11 years

@SoneshDabhi When you close the BufferedReader it closes InputStreamreader which closes the InputStream you got from openStream() etc.
Maksim Dmitriev about 11 years

@aetheria, thank you. Anyhow I need a good Java streams tutorial.
Kasun Siyambalapitiya over 7 years

@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
ᴇʟᴇvᴀтᴇ over 7 years

Kasun, I suggest you post a separate question with an example.
Krzysztof Jabłoński about 6 years

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.
ᴇʟᴇvᴀтᴇ about 6 years

No, this is a misreading. There are two try blocks. The variable in is not defined inside the try block that closes it.
Krzysztof Jabłoński about 6 years

@ᴇʟᴇvᴀтᴇ You're right. Something must be wrong with me then. Sorry for confusion.