[JGIT PATCH 07/10] Rescan packs if a pack is modified in-place (part 1)

["Shawn O. Pearce" <spearce@spearce.org>]

If a pack file is modified in place it usually means that some other
process has repacked this repository, but the contents of this one
pack remained the same, but the object offsets may be different due
to different pack creation settings.  In such cases we can no longer
use the existing PacKFile or PackIndex instances and instead we need
to create new ones.

This isn't a full solution to the problem.  It is possible for an
application to obtain a PackedObjectLoader, see the PackFile get
closed due to pressure on the WindowCache, then fail when the pack
is reopened due to the pack being recreated on disk.  PackWriter
is very susceptible to this, as it caches PackedObjectLoaders for
a long time before it uses them to copy data out of an existing
pack file.

This particular solution only catches the failure where we had
opened the index a long time ago, closed the pack, and are now
opening it again in order to construct a PackedObjectLoader for
the caller.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/ObjectDirectory.java  |   66 +++++++++++++------
 .../src/org/spearce/jgit/lib/PackFile.java         |   20 +++++-
 2 files changed, 63 insertions(+), 23 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
index e7156c4..36f221e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
@@ -51,6 +51,7 @@
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.spearce.jgit.errors.PackMismatchException;
 import org.spearce.jgit.util.FS;
 
 /**
@@ -190,38 +191,53 @@ protected boolean hasObject1(final AnyObjectId objectId) {
 	@Override
 	protected ObjectLoader openObject1(final WindowCursor curs,
 			final AnyObjectId objectId) throws IOException {
-		for (final PackFile p : packs()) {
-			try {
-				final ObjectLoader ldr = p.get(curs, objectId);
-				if (ldr != null) {
-					return ldr;
+		PackFile[] pList = packs();
+		SEARCH: for (;;) {
+			for (final PackFile p : pList) {
+				try {
+					final ObjectLoader ldr = p.get(curs, objectId);
+					if (ldr != null) {
+						return ldr;
+					}
+				} catch (PackMismatchException e) {
+					// Pack was modified; refresh the entire pack list.
+					//
+					pList = scanPacks(pList);
+					continue SEARCH;
+				} catch (IOException e) {
+					// Assume the pack is corrupted.
+					//
+					removePack(p);
 				}
-			} catch (IOException e) {
-				// Assume the pack is corrupted.
-				//
-				removePack(p);
-				continue;
 			}
+			return null;
 		}
-		return null;
 	}
 
 	@Override
 	void openObjectInAllPacks1(final Collection<PackedObjectLoader> out,
 			final WindowCursor curs, final AnyObjectId objectId)
 			throws IOException {
-		for (final PackFile p : packs()) {
-			try {
-				final PackedObjectLoader ldr = p.get(curs, objectId);
-				if (ldr != null) {
-					out.add(ldr);
+		PackFile[] pList = packs();
+		SEARCH: for (;;) {
+			for (final PackFile p : pList) {
+				try {
+					final PackedObjectLoader ldr = p.get(curs, objectId);
+					if (ldr != null) {
+						out.add(ldr);
+					}
+				} catch (PackMismatchException e) {
+					// Pack was modified; refresh the entire pack list.
+					//
+					pList = scanPacks(pList);
+					continue SEARCH;
+				} catch (IOException e) {
+					// Assume the pack is corrupted.
+					//
+					removePack(p);
 				}
-			} catch (IOException e) {
-				// Assume the pack is corrupted.
-				//
-				removePack(p);
-				continue;
 			}
+			break SEARCH;
 		}
 	}
 
@@ -350,6 +366,14 @@ synchronized (packList) {
 	private static Map<String, PackFile> reuseMap(final PackFile[] old) {
 		final Map<String, PackFile> forReuse = new HashMap<String, PackFile>();
 		for (final PackFile p : old) {
+			if (p.invalid()) {
+				// The pack instance is corrupted, and cannot be safely used
+				// again. Do not include it in our reuse map.
+				//
+				p.close();
+				continue;
+			}
+
 			final PackFile prior = forReuse.put(p.getPackFile().getName(), p);
 			if (prior != null) {
 				// This should never occur. It should be impossible for us
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
index bda4843..74ffef8 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java
@@ -73,6 +73,8 @@ public int compare(final PackFile a, final PackFile b) {
 
 	private int packLastModified;
 
+	private volatile boolean invalid;
+
 	private PackIndex loadedIdx;
 
 	private PackReverseIndex reverseIdx;
@@ -91,14 +93,24 @@ public PackFile(final File idxFile, final File packFile) {
 		pack = new WindowedFile(packFile) {
 			@Override
 			protected void onOpen() throws IOException {
-				onOpenPack();
+				try {
+					onOpenPack();
+				} catch (IOException e) {
+					invalid = true;
+					throw e;
+				}
 			}
 		};
 	}
 
 	private synchronized PackIndex idx() throws IOException {
 		if (loadedIdx == null) {
-			loadedIdx = PackIndex.open(idxFile);
+			try {
+				loadedIdx = PackIndex.open(idxFile);
+			} catch (IOException e) {
+				invalid = true;
+				throw e;
+			}
 		}
 		return loadedIdx;
 	}
@@ -267,6 +279,10 @@ boolean supportsFastCopyRawData() throws IOException {
 		return idx().hasCRC32Support();
 	}
 
+	boolean invalid() {
+		return invalid;
+	}
+
 	private void onOpenPack() throws IOException {
 		final PackIndex idx = idx();
 		final WindowCursor curs = new WindowCursor();
-- 
1.6.3.rc1.188.ga02b