CODEWITHSUNDEEP
javajava-io
kostrykin
kostrykin

Reputation: 4292

Resource leak: 'in' is never closed, though it IS closed

I know that there are a couple of similarly entitled questions out there, but most of them have simply forgotten to put a close() directive on their stream. This here is different.

Lets say I have the following minimal example:

public void test() throws IOException
{
    InputStream in;
    if( file.exists() )
    {
        in = new FileInputStream( file );
    }
    else
    {
        in = new URL( "some url" ).openStream();
    }
    in.close();
}

This give me a Resource leak: 'in' is never closed warning in Eclipse (Juno SR1). But when I move the in.close() into the conditional block, the warnings vanishes:

public void test() throws IOException
{
    InputStream in;
    if( file.exists() )
    {
        in = new GZIPInputStream( new FileInputStream( file ) );
        in.close();
    }
    else
    {
        in = new URL( "some URL" ).openStream();
    }
}

What is going on here?

Upvotes: 10

Views: 12781

Answers (6)

Edwin
Edwin

Reputation: 2278

I have something like:

InputStream content = httpResponse.getEntity()==null?null:httpResponse.getEntity().getContent();

that gives the same warrning. But if I leave it just like this:

InputStream content =httpResponse.getEntity().getContent();

I receive no warrnings. Isn't strange or what?

-- I hope my info is adding knowledge to the original question. Thanks!

Upvotes: 0

demongolem
demongolem

Reputation: 9708

This same Eclipse reporting can happen when you explicitly throw an exception after you have opened your resource like:

public void method() throws IOException {
   BufferedReader br = new BufferedReader(new FileReader("myfile.txt"));
   while (br.ready()) {
      String line = br.readLine():
      if (line.length() > 255) {
         throw new IOException("I am some random IOException");
      }
   }
   br.close();
}

This is some contrived code for demonstration purposes so don't look too hard.

If one were to comment out the line, the warning goes away. Of course, you instead want to make sure that that resource is being closed properly. You could do:

if (line.length() > 255) {
   br.close();
   throw new IOException("I am some random IOException");
}

Do not rely on the Eclipse warnings in this case though. Get in the habit of using the try/finally approach to make sure that resources are correctly and consistently being closed.

Upvotes: 0

Dave G
Dave G

Reputation: 9777

Because of the IO exception, you can run into a resource leak (poentially)

Try doing the following:

public void test() throws IOException
{
    InputStream in= null;
    try {
        if( file.exists() )
        {
            // In this case, if the FileInputStream call does not
            // throw a FileNotFoundException (descendant of IOException)
            // it will create the input stream which you are wrapping
            // in a GZIPInputStream (no IO exception on construction)
            in = new GZIPInputStream( new FileInputStream( file ) );
        }
        else
        {
            // Here however, if you are able to create the URL
            // object, "some url" is a valid URL, when you call
            // openStream() you have the potential of creating
            // the input stream. new URL(String spec) will throw
            // a MalformedURLException which is also a descendant of
            // IOException.
            in = new URL( "some url" ).openStream();
        }

        // Do work on the 'in' here 
    } finally {
        if( null != in ) {
            try 
            {
                in.close();
            } catch(IOException ex) {
                // log or fail if you like
            }
        }
    }
}

Doing the above will make sure you've closed the stream or at least made a best effort to do so.

In your original code, you had the InputStream declared but never initialized. That is bad form to begin with. Initialize that to null as I illustrated above. My feeling, and I'm not running Juno at the moment, is that it sees that the InputStream 'in', may potentially make it through all the hoops and hurdles to get to the point at which you are going to use it. Unfortunate, as someone pointed out, your code is a bit dodgy for an example. Doing this as I've detailed as well as @duffymo you'll get rid of the warning.

Upvotes: 7

duffymo
duffymo

Reputation: 308763

Here's how I'd write it:

public void test() throws IOException
{
    InputStream in = null;
    try {
        if(file.exists()) {
            in = new FileInputStream( file );
        } else {
            in = new URL( "some url" ).openStream();
        }
        // Do something useful with the stream.
    } finally {
        close(in);
    }
}

public static void close(InputStream is) {
    try {
        if (is != null) {
            is.close();
        }
    } catch (IOException e) {
        e.printStackTrace();
    }
}

Upvotes: 5

Wade Pedersen
Wade Pedersen

Reputation: 38

Your in stream may not be initialized if the file doesn't exist and you try to close a non-existent file.

Your second example would also need a close statement to avoid leaks.

Upvotes: 0

Peter Lawrey
Peter Lawrey

Reputation: 533530

I suspect the warning is incorrect. It could be checking you are closing the stream in the same scope. In the second case, you are not closing the second stream.

Upvotes: 4

Related Questions