Which resources should be closed?

user_11321646 0
Viewed 276

I'm trying to download a file from an URL. There are many resources and I dont know which one I need to close or do I simply have to close all of them?

    public void downloadUpdate(final String url) {
    try {

        /* Which of these resources do I need to close? */
        final InputStream inputStream = new URL(url).openStream();
        final ReadableByteChannel readableByteChannel = Channels.newChannel(inputStream);
        final FileOutputStream fileOutputStream = new FileOutputStream(Bukkit.getServer().getUpdateFolderFile());
        final FileChannel fileChannel = fileOutputStream.getChannel();

        /* Downloading the update... */
        fileChannel.transferFrom(readableByteChannel, 0, Long.MAX_VALUE);

    } catch (final IOException exception) {
        exception.printStackTrace();
    }
}
download file java
3 Answers

In your case, likely the only resources that need to be closed are the InputStream and FileOutputStream. However, why not simply close them all by using try-with-resources? It doesn't hurt to call Closeable#close() just in case1. In fact, you probably should close every Closeable in your control (i.e. that you opened) when finished with them (you don't necessarily know if a wrapper also needs to release resources).

try (InputStream inputStream = new URL(url).openStream();
     ReadableByteChannel readableByteChannel = Channels.newChannel(inputStream);
     FileOutputStream fileOutputStream = new FileOutputStream(Bukkit.getServer().getUpdateFolderFile());
     FileChannel fileChannel = fileOutputStream.getChannel()) {

    fileChannel.transferFrom(readableByteChannel, 0, Long.MAX_VALUE);

} catch (IOException ex) {
    ex.printStackTrace();
}

The above is not much different than what you currently have.

1. Calling Closeable#close() method has no effect if invoked previously. This isn't necessarily true of AutoCloseable implementations (that don't implement Closeable).

Also, if you're using Java 9+ you don't need to deal with NIO channels. The InputStream class had a method added in Java 9: transferTo(OutputStream).

try (InputStream is = new URL(url).openStream();
     FileOutputStream fos = new FileOutputStream(...)) {
    is.transferTo(fos);
} catch (IOException ex) {
    ex.printStackTrace();
}

You might also be able to use Files.copy(InputStream,Path,CopyOption...) (Java 7+).

try (InputStream is = new URL(url).openStream()) {
    Path file = Bukkit.getServer().getUpdateFolderFile().toPath();
    Files.copy(is, file, StandardCopyOption.REPLACE_EXISTING);
} catch (IOException ex) {
    ex.printStackTrace();
}

Note this might not result in the same behavior as when using FileOutputStream. I'm not entirely sure if FileOutputStream#<init>(File) truncates any existing bytes or if it simply overwrites the bytes from the beginning.

If you prefer using NIO channels, you can open a FileChannel directly via FileChannel.open(Path,OpenOption...) rather than going through a FileOutputStream. As I showed in an example above, you can convert a File into a Path using File#toPath().

user_63956270

In your case it should be enough to close FileChannel, and it's gonna close parent, or FileDescriptor (see java11 sources) of according resource. Actually, with the same result you can close only fileOutputStream and it's gonna release all file resource as well (source code)

But with that you're relying on the implementation of these closable resources, so the best practice is to close each resource you have opened, and easier way to do so is to use try with resources.

user_101443090

Thanks guys, since I'm using Java 8, I'm probably gonna use Files.copy(InputStream in, Path target, CopyOption... options) and just close the InputStream. Otherwise I would've used InputStream#transferTo​(OutputStream out).

user_113216460
Related