Suppress resource leaks warnings in FileUtils #283
Conversation
| @@ -68,18 +68,16 @@ public static String read(URL location, String encoding) throws IOException { | |||
| public static String[] read(final URL[] locations, final String encoding) throws IOException { | |||
| final String[] results = new String[locations.length]; | |||
| for (int i = 0; i < locations.length; i++) { | |||
| final Reader reader = new BufferedReader(new InputStreamReader(locations[i].openStream(), encoding)); | |||
| try { | |||
| try (Reader reader = new BufferedReader(new InputStreamReader(locations[i].openStream(), encoding))) { | |||
| try (FileChannel channel = new FileOutputStream(file).getChannel()) { | ||
| channel.write(ByteBuffer.wrap(content.getBytes(encoding))); | ||
| try (FileOutputStream outStream = new FileOutputStream(file)) { | ||
| outStream.getChannel().write(ByteBuffer.wrap(content.getBytes(encoding))); |
There was a problem hiding this comment.
Please revert: there was no leak here: the construction is right: we can convert a FileOutputStream to a FileChannel and only use and close the latter.
FileChannel implements Channel, which is Closeable. Besides, the javadoc clearly states that the channel will be closed, and as you can see in the FileChannel impl, its file descriptor is released and closed.
Regarding the usage of FileOutputStream and FileChannel, closing FileOutputStream will also close the channel, as stated in the javadoc of its close() method, and closing the FileChannel will also close the underlying file descriptor. Closing one or the other is fine.
This construction new FileOutputStream(file).getChannel() also prevents anyone from using the FileOutputStream and allows it to be GC-ed as soon as possible.
Side note: if there were such a leak in the plugin, that is running on very big projects, I think that it would have been discovered years go ;-)
There was a problem hiding this comment.
Indeed you're right. I'd looked for close() and didn't see it but didn't look at the inheritance.
Side note: if there were such a leak in the plugin, that is running on very big projects, I think that it would have been discovered years go ;-)
Yeah, probably true! Got too eager trying to get rid of yellow triangles in my IDE. :-)
There was a problem hiding this comment.
Reverted and added some suppressed warnings to help the future me.
license-maven-plugin/src/main/java/com/mycila/maven/plugin/license/util/FileUtils.java
Outdated
Show resolved
Hide resolved
license-maven-plugin/src/main/java/com/mycila/maven/plugin/license/util/FileUtils.java
Outdated
Show resolved
Hide resolved
dbwiddis
force-pushed
the
resourceleak
branch
from
6282442
to
479bda2
Compare
dbwiddis
changed the title
dbwiddis
changed the title
FileChannelis notCloseable,so including it in a try-with-resources block does not close the underling streams. Refactored to ensure the closeable object is referenced in try.Also implemented a try-with-resources block for the
BufferedReaderclosure.