Search code examples
javabufferedinputstreamresource-leak

Is there really a resource leak if I close the underlying stream?


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?


Solution

  • There is no resource leak in this case. However, the cost of closing the BufferedInputStream instead (or as well) is minimal, so it is simplest to add a (strictly) unnecessary close to make the analysis tool happy.

    Typical static analysis tools are looking for structural patterns in the code that indicate bugs. In this case, the pattern matching approach is giving a false positive for a resource leak.