CODEWITHSUNDEEP
javabufferedreaderfilereaderresource-leak
Georgios Tasioulis
Georgios Tasioulis

Reputation: 37

Resource leak in my java program

I am trying to write a method in java, where I take some information from a file and see if the file has the information the user looks for. However, for the code that I present, eclipse signs that I have an resource leak in line "return true;" and that the "br = new BufferedReader(fr);" is never close, despite the fact that I am using the close() method at the end of the program. Apparently I am missing something. Could someone help me figure out what is happening? Great thanks in advance!

import java.io.*;

class Help{
    String helpfile;

    Help(String fname){
        helpfile = fname;
    }

    boolean helpon(String what){
        FileReader fr;
        BufferedReader br;
        int ch;
        String topic, info;

        try{
            fr = new FileReader(helpfile);
            br = new BufferedReader(fr);
        }
        catch(FileNotFoundException e){
            System.out.println(e);
            return false;
        }

        try{
            do{
                ch = br.read();
                if(ch=='#'){
                    topic = br.readLine();
                    if(what.compareTo(topic) == 0){
                        do{
                            info = br.readLine();
                            if(info!=null)
                                System.out.println(info);
                        }while((info!= null) && (info.compareTo("")!= 0));
                        return true;
                    }
                }
            }while(ch!=-1);
        }
        catch(IOException e){
            System.out.println(e);
            return false;
        }

        try{
            br.close();
        }
        catch(IOException e){
            System.out.println(e);
        }
        return false;
    }
}

Upvotes: 1

Views: 1801

Answers (4)

Dan W
Dan W

Reputation: 5782

The issue is that you're returning before the program gets the chance to close the resource. There are 2 ways to fix this issue:

  1. Put the returns after you close the resource (by possibly putting the return result in a boolean).
  2. Modify your code to put the close in a finally block so any return done will still execute that code.

Number 2 is generally a more accepted practice because then if you add more things in the future you are still guaranteed to close the resource (unless a catastrophic event occurs).

boolean helpon(String what){
    FileReader fr;
    BufferedReader br;
    int ch;
    String topic, info;

    try{
        fr = new FileReader(helpfile);
        br = new BufferedReader(fr);
        do{
            ch = br.read();
            if(ch=='#'){
                topic = br.readLine();
                if(what.compareTo(topic) == 0){
                    do{
                        info = br.readLine();
                        if(info!=null)
                            System.out.println(info);
                    }while((info!= null) && (info.compareTo("")!= 0));
                    return true;
                }
            }
        }while(ch!=-1);
    } catch(IOException e){
        System.out.println(e);
        return false;
    } catch(FileNotFoundException e){
        System.out.println(e);
        return false;
    } finally {
        try {
            if (br != null) {
                br.close();
            }
        } catch(IOException e){
            System.out.println(e);
            return false;
        }
    }
}

Upvotes: 3

Hypino
Hypino

Reputation: 1248

You have return statements all over the method, but only have a br.close() at the end. It's possible in the flow of code that the method will be returned leaving the br still open.

You may be interested in using try with resources

try (
        FileReader fr = new FileReader(helpfile);
        BufferedReader br = new BufferedReader(fr)
    ) 
{
  //your code
}
catch (IOException e)
{
 //error
}

With this, the close methods will automatically be called for you on the resources.

Upvotes: 2

pauljwilliams
pauljwilliams

Reputation: 19225

If you're using Java 7, use the try-with-resources functionality:

http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html

Upvotes: 0

colti
colti

Reputation: 393

You should put the call to close() in a finally block. In the current state, your code will never reach the final try/catch because you are returning true or false.

try {

    fr = new FileReader(helpfile);
    br = new BufferedReader(fr);

    do {
        ch = br.read();
        if(ch=='#'){
            topic = br.readLine();
            if(what.compareTo(topic) == 0){
                do{
                    info = br.readLine();
                    if(info!=null)
                        System.out.println(info);
                }while((info!= null) && (info.compareTo("")!= 0));
                return true;
            }
        }
    }while(ch!=-1);

} catch (IOException e) {
    System.out.println(e);
    return false;

} catch (FileNotFoundException e) {
    System.out.println(e);
    return false;

} finally  {

    try {

        if (br != null) {
            br.close();
        }

    } catch (IOException e) {
        System.out.println(e);
    }

}

Upvotes: 1

Related Questions