Thanks again Alex

From: Alex Osborne <notifications@github.com<mailto:notifications@github.com>>
Reply-To: nla/amberdb <reply@reply.github.com<mailto:reply@reply.github.com>>
Date: Monday, 17 February 2014 3:29 PM
To: nla/amberdb <amberdb@noreply.github.com<mailto:amberdb@noreply.github.com>>
Cc: Shaun Coen <scoen@nla.gov.au<mailto:scoen@nla.gov.au>>
Subject: Re: [amberdb] Major refactor (#100)


In src/amberdb/model/Copy.java:

> @@ -333,6 +333,7 @@ private long copyBlobToFile(Blob blob, Path destinationFile) throws IOException
>                      StandardOpenOption.CREATE,
>                      StandardOpenOption.TRUNCATE_EXISTING);
>              long bytesTransferred = dest.transferFrom(channel, 0, Long.MAX_VALUE);
> +            dest.close();


channel needs to be closed too. The close MUST also be in a finally block so if an exception is thrown (eg the disk fills up, or there's a read error or permissions problem etc etc) and the transferFrom or second open fails the file handles don't leak. Either:

try (ReadableByteChannel channel = blob.openChannel();
      FileChannel dest = FileChannel.open(...)) {

}


or

ReadableByteChannel channel = blob.openChannel();
try {
  FileChannel dest = FileChannel.open(...);
  try {

  } finally {
    dest.close();
  }
} finally {
  channel.close();
}


I generally much prefer the former (Java 7 syntax) as it's both cleaner and less prone to error (eg people adding statements between the open and try).

—
Reply to this email directly or view it on GitHub<https://github.com/nla/amberdb/pull/100/files#r9782090>.