Do not reuse packfiles when changed on filesystem

The pack reload mechanism from the filesystem works only by name
and does not check the actual last modified date of the packfile.

This lead to concurrency issues where multiple threads were loading
and removing from each other list of packfiles when one of those
was failing the checksum.

Rely on FileSnapshot rather than directly checking lastModified
timestamp so that more checks can be performed.

Bug: 544199
Change-Id: I173328f29d9914007fd5eae3b4c07296ab292390
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>

2 files changed, 15 insertions, 2 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
index 6489415ebb..44ad99bb9a 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
@@ -820,13 +820,13 @@ public class ObjectDirectory extends FileObjectDatabase {
 			}
 
 			final String packName = base + PACK.getExtension();
+			final File packFile = new File(packDirectory, packName);
 			final PackFile oldPack = forReuse.remove(packName);
-			if (oldPack != null) {
+			if (oldPack != null && oldPack.getFileSnapshot().isModified(packFile)) {
 				list.add(oldPack);
 				continue;
 			}
 
-			final File packFile = new File(packDirectory, packName);
 			list.add(new PackFile(packFile, extensions));
 			foundNew = true;
 		}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java
index b5889f2dd4..9a1d12f407 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java
@@ -129,6 +129,8 @@ public class PackFile implements Iterable<PackIndex.MutableEntry> {
 
 	int packLastModified;
 
+	private FileSnapshot fileSnapshot;
+
 	private volatile boolean invalid;
 
 	private boolean invalidBitmap;
@@ -163,6 +165,7 @@ public class PackFile implements Iterable<PackIndex.MutableEntry> {
 	public PackFile(final File packFile, int extensions) {
 		this.packFile = packFile;
 		this.packLastModified = (int) (packFile.lastModified() >> 10);
+		this.fileSnapshot = FileSnapshot.save(packFile);
 		this.extensions = extensions;
 
 		// Multiply by 31 here so we can more directly combine with another
@@ -335,6 +338,16 @@ public class PackFile implements Iterable<PackIndex.MutableEntry> {
 		return getReverseIdx().findObject(offset);
 	}
 
+	/**
+	 * Return the @{@link FileSnapshot} associated to the underlying packfile
+	 * that has been used when the object was created.
+	 *
+	 * @return the packfile @{@link FileSnapshot} that the object is loaded from.
+	 */
+	FileSnapshot getFileSnapshot() {
+		return fileSnapshot;
+	}
+
 	private final byte[] decompress(final long position, final int sz,
 			final WindowCursor curs) throws IOException, DataFormatException {
 		byte[] dstbuf;