# HG changeset patch
# User Artem Tikhomirov <tikhomirov.artem@gmail.com>
# Date 1368821063 -7200
# Node ID 868b2ffdcd5c0acfa164b333533e0892779709a6
# Parent  7c0d2ce340b8648077b9b97f9028b2f9e94dc675
Close FIS, not FileChannel, to clear both references to FileDescriptor right away

diff -r 7c0d2ce340b8 -r 868b2ffdcd5c src/org/tmatesoft/hg/internal/CommitFacility.java
--- a/src/org/tmatesoft/hg/internal/CommitFacility.java	Thu May 16 19:46:13 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/CommitFacility.java	Fri May 17 22:04:23 2013 +0200
@@ -196,8 +196,8 @@
 		}
 		String oldBranchValue = DirstateReader.readBranch(repo);
 		String newBranchValue = branch == null ? DEFAULT_BRANCH_NAME : branch;
-		// TODO undo.dirstate and undo.branch as described in http://mercurial.selenic.com/wiki/FileFormats#undo..2A
 		if (!oldBranchValue.equals(newBranchValue)) {
+			// prepare undo.branch as described in http://mercurial.selenic.com/wiki/FileFormats#undo..2A
 			File branchFile = transaction.prepare(repo.getRepositoryFile(Branch), repo.getRepositoryFile(UndoBranch));
 			FileOutputStream fos = null;
 			try {
diff -r 7c0d2ce340b8 -r 868b2ffdcd5c src/org/tmatesoft/hg/internal/DataAccessProvider.java
--- a/src/org/tmatesoft/hg/internal/DataAccessProvider.java	Thu May 16 19:46:13 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/DataAccessProvider.java	Fri May 17 22:04:23 2013 +0200
@@ -79,18 +79,18 @@
 			return new DataAccess();
 		}
 		try {
-			FileChannel fc = new FileInputStream(f).getChannel(); // FIXME SHALL CLOSE FIS, not only channel
-			long flen = fc.size();
+			FileInputStream fis = new FileInputStream(f);
+			long flen = f.length();
 			if (!shortRead && flen > mapioMagicBoundary) {
 				// TESTS: bufLen of 1024 was used to test MemMapFileAccess
-				return new MemoryMapFileAccess(fc, flen, mapioBufSize, context.getLog());
+				return new MemoryMapFileAccess(fis, flen, mapioBufSize, context.getLog());
 			} else {
 				// XXX once implementation is more or less stable,
 				// may want to try ByteBuffer.allocateDirect() to see
 				// if there's any performance gain. 
 				boolean useDirectBuffer = false; // XXX might be another config option
 				// TESTS: bufferSize of 100 was used to check buffer underflow states when readBytes reads chunks bigger than bufSize
-				return new FileAccess(fc, flen, bufferSize, useDirectBuffer, context.getLog());
+				return new FileAccess(fis, flen, bufferSize, useDirectBuffer, context.getLog());
 			}
 		} catch (IOException ex) {
 			// unlikely to happen, we've made sure file exists.
@@ -109,6 +109,7 @@
 	}
 
 	private static class MemoryMapFileAccess extends DataAccess {
+		private FileInputStream fileStream;
 		private FileChannel fileChannel;
 		private long position = 0; // always points to buffer's absolute position in the file
 		private MappedByteBuffer buffer;
@@ -116,8 +117,9 @@
 		private final int memBufferSize;
 		private final LogFacility logFacility;
 
-		public MemoryMapFileAccess(FileChannel fc, long channelSize, int bufferSize, LogFacility log) {
-			fileChannel = fc;
+		public MemoryMapFileAccess(FileInputStream fis, long channelSize, int bufferSize, LogFacility log) {
+			fileStream = fis;
+			fileChannel = fis.getChannel();
 			size = channelSize;
 			logFacility = log;
 			memBufferSize = bufferSize > channelSize ? (int) channelSize : bufferSize; // no reason to waste memory more than there's data 
@@ -241,27 +243,26 @@
 		@Override
 		public void done() {
 			buffer = null;
-			if (fileChannel != null) {
-				try {
-					fileChannel.close();
-				} catch (IOException ex) {
-					logFacility.dump(getClass(), Warn, ex, null);
-				}
-				fileChannel = null;
+			if (fileStream != null) {
+				new FileUtils(logFacility).closeQuietly(fileStream);
+				fileStream = null;
+				fileChannel = null; // channel is closed together with stream
 			}
 		}
 	}
 
 	// (almost) regular file access - FileChannel and buffers.
 	private static class FileAccess extends DataAccess {
+		private FileInputStream fileStream;
 		private FileChannel fileChannel;
 		private ByteBuffer buffer;
 		private long bufferStartInFile = 0; // offset of this.buffer in the file.
 		private final long size;
 		private final LogFacility logFacility;
 
-		public FileAccess(FileChannel fc, long channelSize, int bufferSizeHint, boolean useDirect, LogFacility log) {
-			fileChannel = fc;
+		public FileAccess(FileInputStream fis, long channelSize, int bufferSizeHint, boolean useDirect, LogFacility log) {
+			fileStream = fis;
+			fileChannel = fis.getChannel();
 			size = channelSize;
 			logFacility = log;
 			final int capacity = size < bufferSizeHint ? (int) size : bufferSizeHint;
@@ -372,15 +373,10 @@
 
 		@Override
 		public void done() {
-			if (buffer != null) {
-				buffer = null;
-			}
-			if (fileChannel != null) {
-				try {
-					fileChannel.close();
-				} catch (IOException ex) {
-					logFacility.dump(getClass(), Warn, ex, null);
-				}
+			buffer = null;
+			if (fileStream != null) {
+				new FileUtils(logFacility).closeQuietly(fileStream);
+				fileStream = null;
 				fileChannel = null;
 			}
 		}
diff -r 7c0d2ce340b8 -r 868b2ffdcd5c src/org/tmatesoft/hg/internal/FileUtils.java
--- a/src/org/tmatesoft/hg/internal/FileUtils.java	Thu May 16 19:46:13 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/FileUtils.java	Fri May 17 22:04:23 2013 +0200
@@ -34,7 +34,7 @@
  * @author Artem Tikhomirov
  * @author TMate Software Ltd.
  */
-final class FileUtils {
+public final class FileUtils {
 	
 	private final LogFacility log;
 	
diff -r 7c0d2ce340b8 -r 868b2ffdcd5c src/org/tmatesoft/hg/repo/HgDataFile.java
--- a/src/org/tmatesoft/hg/repo/HgDataFile.java	Thu May 16 19:46:13 2013 +0200
+++ b/src/org/tmatesoft/hg/repo/HgDataFile.java	Fri May 17 22:04:23 2013 +0200
@@ -20,7 +20,6 @@
 import static org.tmatesoft.hg.repo.HgInternals.wrongRevisionIndex;
 import static org.tmatesoft.hg.repo.HgRepository.*;
 import static org.tmatesoft.hg.util.LogFacility.Severity.Info;
-import static org.tmatesoft.hg.util.LogFacility.Severity.Warn;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -37,12 +36,12 @@
 import org.tmatesoft.hg.internal.DataAccess;
 import org.tmatesoft.hg.internal.FileHistory;
 import org.tmatesoft.hg.internal.FileRevisionHistoryChunk;
+import org.tmatesoft.hg.internal.FileUtils;
 import org.tmatesoft.hg.internal.FilterByteChannel;
 import org.tmatesoft.hg.internal.FilterDataAccess;
 import org.tmatesoft.hg.internal.Internals;
 import org.tmatesoft.hg.internal.Metadata;
 import org.tmatesoft.hg.internal.RevlogStream;
-import org.tmatesoft.hg.repo.HgBlameInspector;
 import org.tmatesoft.hg.util.ByteChannel;
 import org.tmatesoft.hg.util.CancelSupport;
 import org.tmatesoft.hg.util.CancelledException;
@@ -168,9 +167,10 @@
 			final int bsize = (int) Math.min(flength, 32*1024);
 			progress.start((int) (flength > Integer.MAX_VALUE ? flength >>> 15 /*32 kb buf size*/ : flength));
 			ByteBuffer buf = ByteBuffer.allocate(bsize);
-			FileChannel fc = null;
+			FileInputStream fis = null;
 			try {
-				fc = new FileInputStream(f).getChannel();
+				fis = new FileInputStream(f);
+				FileChannel fc = fis.getChannel();
 				while (fc.read(buf) != -1) {
 					cs.checkCancelled();
 					buf.flip();
@@ -182,12 +182,8 @@
 				throw new HgInvalidFileException("Working copy read failed", ex, f);
 			} finally {
 				progress.done();
-				if (fc != null) {
-					try {
-						fc.close();
-					} catch (IOException ex) {
-						getRepo().getSessionContext().getLog().dump(getClass(), Warn, ex, null);
-					}
+				if (fis != null) {
+					new FileUtils(getRepo().getSessionContext().getLog()).closeQuietly(fis);
 				}
 			}
 		} else {
diff -r 7c0d2ce340b8 -r 868b2ffdcd5c src/org/tmatesoft/hg/repo/HgRepositoryLock.java
--- a/src/org/tmatesoft/hg/repo/HgRepositoryLock.java	Thu May 16 19:46:13 2013 +0200
+++ b/src/org/tmatesoft/hg/repo/HgRepositoryLock.java	Fri May 17 22:04:23 2013 +0200
@@ -191,10 +191,11 @@
 	}
 
 	private static byte[] read(File f) throws IOException {
-		FileChannel fc = new FileInputStream(f).getChannel();
+		FileInputStream fis = new FileInputStream(f);
+		FileChannel fc = fis.getChannel();
 		ByteBuffer bb = ByteBuffer.allocate(Internals.ltoi(fc.size()));
 		fc.read(bb);
-		fc.close();
+		fis.close();
 		return bb.array();
 	}
 }
diff -r 7c0d2ce340b8 -r 868b2ffdcd5c src/org/tmatesoft/hg/util/RegularFileInfo.java
--- a/src/org/tmatesoft/hg/util/RegularFileInfo.java	Thu May 16 19:46:13 2013 +0200
+++ b/src/org/tmatesoft/hg/util/RegularFileInfo.java	Fri May 17 22:04:23 2013 +0200
@@ -66,7 +66,7 @@
 	}
 
 	public int lastModified() {
-		// TODO post-1.0 for symlinks, this returns incorrect mtime of the target file, not that of link itself
+		// TODO [post-1.1] for symlinks, this returns incorrect mtime of the target file, not that of link itself
 		// Besides, timestame if link points to non-existing file is 0.
 		// However, it result only in slowdown in WCStatusCollector, as it need to perform additional content check
 		return (int) (file.lastModified() / 1000);
@@ -84,6 +84,10 @@
 			if (isSymlink()) {
 				return new ByteArrayReadableChannel(getLinkTargetBytes());
 			} else {
+				// TODO [2.0 API break]  might be good idea replace channel with smth 
+				// else, to ensure #close() disposes FileDescriptor. Now
+				// FD has usage count of two (new FileInputStream + getChannel),
+				// and FileChannel#close decrements only 1, second has to wait FIS#finalize() 
 				return new FileInputStream(file).getChannel();
 			}
 		} catch (FileNotFoundException ex) {