I ran a dynamic code analysis tool on one of our programs and this pattern was identified as a resource leak:
...
FileInputStream fileInputStream = new FileInputStream(file);
try {
data = someMethod(new BufferedInputStream(fileInputStream));
// Assume that someMethod(InputStream) internally reads the stream
// until BufferedInputStream.read() returns -1.
...
}
finally {
...
try {
fileInputStream.close();
} catch (IOException e) {
...
}
}
Specifically, the analysis tool marked the new BufferedInputStream(...) call as a resource leak because it is never closed. In this pattern, however, the underlying stream fileInputStream is closed and the BufferedInputStream goes out of scope.
Note: I neglected to make it clear when I originally posted the question, but I realize this is not the "best" implementation. However, if there is not a de-facto resource leak here then it is unlikely that we will scour our legacy codebase for all instances of this pattern and close the outer streams or replace them with newer constructs such as try-with-resources--i.e., "If it ain't broke, don't fix it."
Given this situation, is this actually a resource leak?
Its unlikely this is an actual resource leak, but writing code like this is definitely not a best practice.
In general, you should always call close on the outermost stream, which will ripple to the inner streams.
If you are using Java 7, you can use the new try-with-resources syntax and avoid using a finally block at all.
Related: The try-with-resources Statement
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With