Hand-Closed Resource In Finally
ID |
java.use_try_with_resources |
Severity |
high |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Reliability |
Language |
Java |
Tags |
best-practice, reliability |
Description
Reports a resource that is opened around a plain try and released by a hand-written close() call in the matching finally block. A try-with-resources statement does the same job, and does it correctly by construction.
The rule fires only when the same variable is both allocated (inside the guarded block, or in the run of declarations immediately before the try) and closed in the finally. That pairing is the evidence that the method owns the resource and can therefore move it into the resource list.
Rationale
String firstLine(String path) throws IOException {
FileReader chars = null;
BufferedReader lines = null;
try {
chars = new FileReader(path);
lines = new BufferedReader(chars);
return lines.readLine();
} finally {
if (lines != null) {
lines.close();
}
if (chars != null) {
chars.close();
}
}
}Hand-written teardown is not merely verbose, it is easy to get wrong, and every mistake shows up only on the exceptional path — the one nobody exercises:
-
the close call needs a null guard, because the resource may never have been assigned;
-
the
IOExceptionthatclose()can itself throw needs catching, or it replaces the exception that was already leaving the block and the original cause is lost; -
resources must be released in the reverse order they were acquired.
A try-with-resources statement gets all three right, and a failure raised while closing is suppressed onto the primary exception rather than swallowed or substituted for it.
Four shapes are deliberately left alone:
-
a
close()on something the method never allocated — a parameter, a field, or a value handed over by a factory. The method may not own the resource, and moving it into a resource list would close it on the owner’s behalf; -
a resource allocated before some other statement runs, since hoisting it into the resource list would drag that statement inside the guarded block and change what the method does;
-
a
trywith nofinally, where the resource may simply be leaked rather than closed in the wrong place; -
a resource produced by a factory call rather than by
new, which cannot be told apart from any other call without a type hierarchy.
Remediation
Move the declarations into the resource list. The finally block disappears with them, and both resources are closed in reverse order whatever happens inside:
String firstLine(String path) throws IOException {
try (FileReader chars = new FileReader(path);
BufferedReader lines = new BufferedReader(chars)) {
return lines.readLine();
}
}Any type implementing AutoCloseable — or java.io.Closeable, which extends it — can appear in the resource list. From Java 9 onward an effectively final variable declared just above the statement can be named directly: try (chars) { … }.