Mercurial > jhg

changeset 619:868b2ffdcd5c

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Close FIS, not FileChannel, to clear both references to FileDescriptor right away
author Artem Tikhomirov <tikhomirov.artem@gmail.com>
date Fri, 17 May 2013 22:04:23 +0200
parents 7c0d2ce340b8
children 272ecffccc8a
files src/org/tmatesoft/hg/internal/CommitFacility.java src/org/tmatesoft/hg/internal/DataAccessProvider.java src/org/tmatesoft/hg/internal/FileUtils.java src/org/tmatesoft/hg/repo/HgDataFile.java src/org/tmatesoft/hg/repo/HgRepositoryLock.java src/org/tmatesoft/hg/util/RegularFileInfo.java
diffstat 6 files changed, 36 insertions(+), 39 deletions(-) [+]
line wrap: on
line diff
--- 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 {
--- 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;
 			}
 		}
--- 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;
 	
--- 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 {
--- 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();
 	}
 }
--- 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) {