Resource leak: 'in' is never closed, though it IS closed
Solution 1
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();
}
}
Solution 2
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.
Solution 3
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.
theV0ID
Projects I've been working on lately: Carna (2010 – 2015) cnvmats.py (2015 - 2016)
Updated on July 18, 2022Comments
-
theV0ID almost 2 years
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 thein.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?
-
theV0ID over 11 yearsThanks, this really does work. However, I still don't understand how it is going to happen that
in != null
AND an exception is thrown by theFileInputStream
constructor. Lets assume, theFileInputStream
constructor throws an exception. This implies thatin
is still null. Therefore, theis.close()
statement is ignored either way? -
theV0ID over 11 yearsThanks, this works. However I have an understanding problem with this solution. It is the same as with this answer here: link Can you explain this?
-
duffymo over 11 yearsI'd prefer an upvote and acceptance to your thanks. Your example is contrived, because you're doing nothing else but calling the constructor. But if you were actually using the InputStream it could throw an exception if the read fails. And nothing is "ignored". That's not how programs work.
-
user207421 over 11 yearsI agree with this answer and I have upvoted the pointless and unexplained downvote. The warning is clearly incorrect. The second example where there really is a missing close proves the point.
-
user207421 over 11 yearsNo. If 'in' cant be initialized an exception is thrown and in.close() is not reached. It is not possible for this code to 'try to close a non-existent file'.
-
dramzy over 7 yearsStarting with Java 7, you can just use try with resources.
-
Enerccio over 6 yearsThis is bad answer because if there is nothing to do then it should not throw resource warning, it is a bug in the compiler
-
duffymo over 6 years"throw resource warning"? What are you talking about? The close method throws a checked exception - you have no choice but to deal with it. Why don't you post your answer so we can see what you have to say? This question is almost 8 years old. Is there nothing you can do to boost your rep than finding old answers and posting trollish comments?