Should I close the FileChannel?

Question

I came across an issue with one of our utility classes today. It is a helper for files and contains some static file copy routines. Below are the relevant methods extracted along with a test method.

The problem is that sometimes the setLastModified call fails, returning false.

On my PC (Windows 7, latest Java) I sometimes get the "setLastModified failed" message (About 25 times out of 1000).

I have worked around the problem right now by removing the FileChannel.close calls but I would much prefer to understand why this is happening, even if that is the correct solution.

Does anyone else get the same problem?

private void testCopy() throws FileNotFoundException, IOException {
  File src = new File("C:\\Public\\Test-Src.txt");
  File dst = new File("C:\\Public\\Test-Dst.txt");

  for (int i = 0; i < 1000; i++) {
    copyFile(src, dst);
  }
}

public static void copyFile(final File from, final File to) throws FileNotFoundException, IOException {
  final String tmpName = to.getAbsolutePath() + ".tmp";
  // Copy to a .tmp file.
  final File tmp = new File(tmpName);
  // Do the transfer.
  transfer(from, tmp);
  // Preserve time.
  if (!tmp.setLastModified(from.lastModified())) {
    System.err.println("setLastModified failed!");
  }
  // In case there's one there already.
  to.delete();
  // Rename it in.
  tmp.renameTo(to);
}

public static void transfer(final File from, final File to) throws IOException {
  FileInputStream in = null;
  FileOutputStream out = null;
  try {
    in = new FileInputStream(from);
    out = new FileOutputStream(to);
    transfer(in, out);
  } finally {
    if (null != in) {
      in.close();
    }
    if (null != out) {
      out.close();
    }
  }
}

public static void transfer(final FileInputStream from, final FileOutputStream to) throws IOException {
  FileChannel srcChannel = null;
  FileChannel dstChannel = null;
  //try {
    srcChannel = from.getChannel();
    dstChannel = to.getChannel();
    srcChannel.transferTo(0, srcChannel.size(), dstChannel);
  //} finally {
  //  if (null != dstChannel) {
  //    dstChannel.close();
  //  }
  //  if (null != srcChannel) {
  //    srcChannel.close();
  //  }
  }
}

Edit: I have changed the code to only close the Streamss and not the FileChannels because research suggests closing the FileChannel also closes the Stream.

Answer 1

After some research amongst the various sites that hold java library sources it looks very much like FileChannel.close ultimately calls the FileInputStream.close or FileOutputStream.close of its parent object.

This suggests to me that you should either close the FileChannel or the Stream but not both.

In view of this I am changing my original post to reflect one correct method, i.e. close the Streams not the Channels.

Answer 2

If you are using Java 7, you could use Files.copy(Path source, Path target, CopyOption... options) for this operation to avoid writing, testing and debugging your own implementation.

Alternatively, consider using an external library such as Apache Commons IO. Specifically, you will find FileUtils.copyFile(File srcFile, File destFile) interesting:

/** 
 * Copies a file to a new location preserving the file date.
 * [...]
 * @param srcFile  an existing file to copy, must not be <code>null</code>
 * @param destFile  the new file, must not be <code>null</code>
 * [...]
 */
public static void copyFile(File srcFile, File destFile) throws IOException