FileBasedConfig: Use FileSnapshot for isOutdated()

Relying only on the last modified time for a file can be tricky.
The "racy git" problem may cause some modifications to be missed.

Use the new FileSnapshot code to track when a configuration file
has been modified, and needs to be reloaded in memory.

Change-Id: Ib6312fdd3b2403eee5af3f8ae711294b0e5f9035
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java

@@ -58,6 +58,7 @@ import org.eclipse.jgit.JGitText;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.IO;
@@ -68,8 +69,9 @@ import org.eclipse.jgit.util.RawParseUtils;
  */
 public class FileBasedConfig extends StoredConfig {
 	private final File configFile;
-	private volatile long lastModified;
 	private final FS fs;
+	private volatile FileSnapshot snapshot;
+	private volatile ObjectId hash;
 
 	/**
 	 * Create a configuration with no default fallback.
@@ -99,6 +101,8 @@ public class FileBasedConfig extends StoredConfig {
 		super(base);
 		configFile = cfgLocation;
 		this.fs = fs;
+		this.snapshot = FileSnapshot.DIRTY;
+		this.hash = ObjectId.zeroId();
 	}
 
 	@Override
@@ -125,11 +129,24 @@ public class FileBasedConfig extends StoredConfig {
 	 */
 	@Override
 	public void load() throws IOException, ConfigInvalidException {
-		lastModified = getFile().lastModified();
+		final FileSnapshot oldSnapshot = snapshot;
+		final FileSnapshot newSnapshot = FileSnapshot.save(getFile());
 		try {
-			fromText(RawParseUtils.decode(IO.readFully(getFile())));
+			final byte[] in = IO.readFully(getFile());
+			final ObjectId newHash = hash(in);
+			if (hash.equals(newHash)) {
+				if (oldSnapshot.equals(newSnapshot))
+					oldSnapshot.setClean(newSnapshot);
+				else
+					snapshot = newSnapshot;
+			} else {
+				fromText(RawParseUtils.decode(in));
+				snapshot = newSnapshot;
+				hash = newHash;
+			}
 		} catch (FileNotFoundException noFile) {
 			clear();
+			snapshot = newSnapshot;
 		} catch (IOException e) {
 			final IOException e2 = new IOException(MessageFormat.format(JGitText.get().cannotReadFile, getFile()));
 			e2.initCause(e);
@@ -157,18 +174,29 @@ public class FileBasedConfig extends StoredConfig {
 		if (!lf.lock())
 			throw new IOException(MessageFormat.format(JGitText.get().cannotLockFile, getFile()));
 		try {
-			lf.setNeedStatInformation(true);
+			lf.setNeedSnapshot(true);
 			lf.write(out);
 			if (!lf.commit())
 				throw new IOException(MessageFormat.format(JGitText.get().cannotCommitWriteTo, getFile()));
 		} finally {
 			lf.unlock();
 		}
-		lastModified = lf.getCommitLastModified();
+		snapshot = lf.getCommitSnapshot();
+		hash = hash(out);
 		// notify the listeners
 		fireConfigChangedEvent();
 	}
 
+	@Override
+	public void clear() {
+		hash = hash(new byte[0]);
+		super.clear();
+	}
+
+	private static ObjectId hash(final byte[] rawText) {
+		return ObjectId.fromRaw(Constants.newMessageDigest().digest(rawText));
+	}
+
 	@Override
 	public String toString() {
 		return getClass().getSimpleName() + "[" + getFile().getPath() + "]";
@@ -179,6 +207,6 @@ public class FileBasedConfig extends StoredConfig {
 	 * than the file on disk
 	 */
 	public boolean isOutdated() {
-		return getFile().lastModified() != lastModified;
+		return snapshot.isModified(getFile());
 	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileSnapshot.java

@@ -104,6 +104,10 @@ public class FileSnapshot {
 		this.cannotBeRacilyClean = notRacyClean(read);
 	}
 
+	long lastModified() {
+		return lastModified;
+	}
+
 	/**
 	 * Check if the path has been modified since the snapshot was saved.
 	 *

org.eclipse.jgit/src/org/eclipse/jgit/storage/file/LockFile.java

@@ -89,11 +89,11 @@ public class LockFile {
 
 	private FileOutputStream os;
 
-	private boolean needStatInformation;
+	private boolean needSnapshot;
 
 	private boolean fsync;
 
-	private long commitLastModified;
+	private FileSnapshot commitSnapshot;
 
 	private final FS fs;
 
@@ -334,12 +334,24 @@ public class LockFile {
 
 	/**
 	 * Request that {@link #commit()} remember modification time.
+	 * <p>
+	 * This is an alias for {@code setNeedSnapshot(true)}.
 	 *
 	 * @param on
 	 *            true if the commit method must remember the modification time.
 	 */
 	public void setNeedStatInformation(final boolean on) {
-		needStatInformation = on;
+		setNeedSnapshot(on);
+	}
+
+	/**
+	 * Request that {@link #commit()} remember the {@link FileSnapshot}.
+	 *
+	 * @param on
+	 *            true if the commit method must remember the FileSnapshot.
+	 */
+	public void setNeedSnapshot(final boolean on) {
+		needSnapshot = on;
 	}
 
 	/**
@@ -442,8 +454,8 @@ public class LockFile {
 	}
 
 	private void saveStatInformation() {
-		if (needStatInformation)
+		if (needSnapshot)
-			commitLastModified = lck.lastModified();
+			commitSnapshot = FileSnapshot.save(lck);
 	}
 
 	/**
@@ -452,7 +464,12 @@ public class LockFile {
 	 * @return modification time of the lock file right before we committed it.
 	 */
 	public long getCommitLastModified() {
-		return commitLastModified;
+		return commitSnapshot.lastModified();
+	}
+
+	/** @return get the {@link FileSnapshot} just before commit. */
+	public FileSnapshot getCommitSnapshot() {
+		return commitSnapshot;
 	}
 
 	/**