Here's some code that you might see since Java 7 appeared. Can you spot the bug?
public List<String> extractData(File file, String encoding) throws IOException { List<String> results = new ArrayList<String>(); try (BufferedReader rdr = new BufferedReader(new InputStreamReader(new FileInputStream(file), encoding))) { // read each line and extract data } return results; }
OK, here's a hint: what if encoding
is invalid?
The answer is that the InputStreamReader
constructor will throw
UnsupportedEncodingException
(which is a subclass of IOException
).
But this exception happens after the FileInputStream
has been created,
but before the body of the try-with-resources statement. Which means that the implicit
finally
in that statement won't get executed, and the file will remain open until
the stream's finalizer
executes.
To solve this problem, you need to break apart the constructor chain, to ensure that the actual resource is assigned to its own variable in the resource clause, and will therefore be closed. There are two ways to approach this: either use multiple resource clauses, or push non-resource constructors into the body of the statement.
In this case, the InputStreamReader
and BufferedReader
are simple
decorator objects: if you neglect to close them, it doesn't change the behavior of the program.
So we can construct them inside the body of the statement:
try (InputStream in = new FileInputStream(file)) { BufferedReader rdr = new BufferedReader(new InputStreamReader(in, encoding)); // ...
Bug solved: regardless of what happens when constructing rdr
, the underlying stream
will be closed; we don't have to worry about a “too many open files” exception.
However, if we try to do something similar with output streams, we introduce a new bug:
try (OutputStream out = new FileOutputStream(file)) { // DON'T DO THIS! BufferedWriter wtr = new BufferedWriter(new OutputStreamWriter(out, encoding)); // ...
The problem with this code is that decorators are important when writing. Unless you close
the BufferedOutputStream
, the data in its buffer won't be written to the file.
On the positive side, you'll see this bug the first time you try to write a file. To solve it
and still use the try-with-resources construct, you need to use multiple resource clauses:
try (OutputStream out = new FileOutputStream(file) ; OutputStreamWriter osw = new OutputStreamWriter(out, encoding) ; BufferedWriter wtr = new BufferedWriter(osw)) { // ...
There you have it, the only safe way to use try-with-resources. If you currently have code with nested constructors, I strongly recommend that you change it. And please change any public documentation that has nested constructors; explaining the bug to someone that is “just following the docs” wastes time that could be used for better things.
No comments:
Post a Comment