From d1aacc415a3336144e4b3b55c402a025e920a031 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 30 Jan 2014 16:25:49 -0800 Subject: [PATCH] Fix MissingObjectException race in ObjectDirectory Johannes Carlsson identified a race condition[1] that can lead to spurious MissingObjectExceptions at read time. If two threads are active inside of ObjectDirectory looking for a packed object and the packList is currently the empty NO_PACKS list, thread A will find no object and eventually consider tryAgain1(). If thread A is put to sleep and this point and thread B also does not find the object, loads the packs, when thread A wakes up its tryAgain1 would return false and the thread never considers the packs. Rework the internal API of ObjectDirectory to keep a handle on the exact PackList that was iterated by thread A, allowing it to always retry walking through the packs if the new PackList is different. This had some ripple effect into the CachedObjectDirectory and the shared FileObjectDatabase interface. The new code should be slightly easier to follow, especially from the perspective of the CachedObjectDirectory trying to minimize the number of open system calls it makes to files matching "$GIT_DIR/objects/??/?x{38}". [1] http://dev.eclipse.org/mhonarc/lists/jgit-dev/msg02401.html Change-Id: I9a1c9d6ad6cb38404b7b9178167b714077561353 --- .../storage/file/CachedObjectDirectory.java | 110 +++--- .../storage/file/FileObjectDatabase.java | 219 +---------- .../internal/storage/file/FileRepository.java | 6 +- .../storage/file/LargePackedDeltaObject.java | 2 +- .../storage/file/ObjectDirectory.java | 359 ++++++++++++------ 5 files changed, 296 insertions(+), 400 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/CachedObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/CachedObjectDirectory.java index eb4f01cac..a81d8ec0e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/CachedObjectDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/CachedObjectDirectory.java @@ -72,11 +72,11 @@ class CachedObjectDirectory extends FileObjectDatabase { * The set that contains unpacked objects identifiers, it is created when * the cached instance is created. */ - private final ObjectIdOwnerMap unpackedObjects = new ObjectIdOwnerMap(); + private ObjectIdOwnerMap unpackedObjects; private final ObjectDirectory wrapped; - private AlternateHandle[] alts; + private CachedObjectDirectory[] alts; /** * The constructor @@ -86,11 +86,15 @@ class CachedObjectDirectory extends FileObjectDatabase { */ CachedObjectDirectory(ObjectDirectory wrapped) { this.wrapped = wrapped; + this.unpackedObjects = scanLoose(); + } + private ObjectIdOwnerMap scanLoose() { + ObjectIdOwnerMap m = new ObjectIdOwnerMap(); File objects = wrapped.getDirectory(); String[] fanout = objects.list(); if (fanout == null) - fanout = new String[0]; + return m; for (String d : fanout) { if (d.length() != 2) continue; @@ -102,12 +106,13 @@ class CachedObjectDirectory extends FileObjectDatabase { continue; try { ObjectId id = ObjectId.fromString(d + e); - unpackedObjects.add(new UnpackedObjectId(id)); + m.add(new UnpackedObjectId(id)); } catch (IllegalArgumentException notAnObject) { // ignoring the file that does not represent loose object } } } + return m; } @Override @@ -121,13 +126,13 @@ public ObjectDatabase newCachedDatabase() { } @Override - FileObjectDatabase newCachedFileObjectDatabase() { - return this; + File getDirectory() { + return wrapped.getDirectory(); } @Override - File getDirectory() { - return wrapped.getDirectory(); + File fileFor(AnyObjectId id) { + return wrapped.fileFor(id); } @Override @@ -145,15 +150,12 @@ Set getShallowCommits() throws IOException { return wrapped.getShallowCommits(); } - @Override - AlternateHandle[] myAlternates() { + private CachedObjectDirectory[] myAlternates() { if (alts == null) { - AlternateHandle[] src = wrapped.myAlternates(); - alts = new AlternateHandle[src.length]; - for (int i = 0; i < alts.length; i++) { - FileObjectDatabase s = src[i].db; - alts[i] = new AlternateHandle(s.newCachedFileObjectDatabase()); - } + ObjectDirectory.AlternateHandle[] src = wrapped.myAlternates(); + alts = new CachedObjectDirectory[src.length]; + for (int i = 0; i < alts.length; i++) + alts[i] = src[i].db.newCachedFileObjectDatabase(); } return alts; } @@ -170,61 +172,53 @@ void resolve(Set matches, AbbreviatedObjectId id) } @Override - boolean tryAgain1() { - return wrapped.tryAgain1(); - } - - @Override - public boolean has(final AnyObjectId objectId) { - return hasObjectImpl1(objectId); - } - - @Override - boolean hasObject1(AnyObjectId objectId) { - return unpackedObjects.contains(objectId) - || wrapped.hasObject1(objectId); + public boolean has(final AnyObjectId objectId) throws IOException { + if (unpackedObjects.contains(objectId)) + return true; + if (wrapped.hasPackedObject(objectId)) + return true; + for (CachedObjectDirectory alt : myAlternates()) { + if (alt.has(objectId)) + return true; + } + return false; } @Override ObjectLoader openObject(final WindowCursor curs, final AnyObjectId objectId) throws IOException { - return openObjectImpl1(curs, objectId); - } - - @Override - ObjectLoader openObject1(WindowCursor curs, AnyObjectId objectId) - throws IOException { - if (unpackedObjects.contains(objectId)) - return wrapped.openObject2(curs, objectId.name(), objectId); - return wrapped.openObject1(curs, objectId); - } - - @Override - boolean hasObject2(String objectId) { - return unpackedObjects.contains(ObjectId.fromString(objectId)); - } - - @Override - ObjectLoader openObject2(WindowCursor curs, String objectName, - AnyObjectId objectId) throws IOException { - if (unpackedObjects.contains(objectId)) - return wrapped.openObject2(curs, objectName, objectId); + ObjectLoader ldr = openLooseObject(curs, objectId); + if (ldr != null) + return ldr; + ldr = wrapped.openPackedObject(curs, objectId); + if (ldr != null) + return ldr; + for (CachedObjectDirectory alt : myAlternates()) { + ldr = alt.openObject(curs, objectId); + if (ldr != null) + return ldr; + } return null; } @Override - long getObjectSize1(WindowCursor curs, AnyObjectId objectId) throws IOException { - if (unpackedObjects.contains(objectId)) - return wrapped.getObjectSize2(curs, objectId.name(), objectId); - return wrapped.getObjectSize1(curs, objectId); + long getObjectSize(WindowCursor curs, AnyObjectId objectId) + throws IOException { + // Object size is unlikely to be requested from contexts using + // this type. Don't bother trying to accelerate the lookup. + return wrapped.getObjectSize(curs, objectId); } @Override - long getObjectSize2(WindowCursor curs, String objectName, AnyObjectId objectId) + ObjectLoader openLooseObject(WindowCursor curs, AnyObjectId id) throws IOException { - if (unpackedObjects.contains(objectId)) - return wrapped.getObjectSize2(curs, objectName, objectId); - return -1; + if (unpackedObjects.contains(id)) { + ObjectLoader ldr = wrapped.openLooseObject(curs, id); + if (ldr != null) + return ldr; + unpackedObjects = scanLoose(); + } + return null; } @Override diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileObjectDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileObjectDatabase.java index ba45334bd..3afc050ba 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileObjectDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileObjectDatabase.java @@ -74,61 +74,6 @@ public ObjectDirectoryInserter newInserter() { return new ObjectDirectoryInserter(this, getConfig()); } - /** - * Does the requested object exist in this database? - *

- * Alternates (if present) are searched automatically. - * - * @param objectId - * identity of the object to test for existence of. - * @return true if the specified object is stored in this database, or any - * of the alternate databases. - */ - public boolean has(final AnyObjectId objectId) { - return hasObjectImpl1(objectId) || hasObjectImpl2(objectId.name()); - } - - /** - * Compute the location of a loose object file. - * - * @param objectId - * identity of the loose object to map to the directory. - * @return location of the object, if it were to exist as a loose object. - */ - File fileFor(final AnyObjectId objectId) { - return fileFor(objectId.name()); - } - - File fileFor(final String objectName) { - final String d = objectName.substring(0, 2); - final String f = objectName.substring(2); - return new File(new File(getDirectory(), d), f); - } - - final boolean hasObjectImpl1(final AnyObjectId objectId) { - if (hasObject1(objectId)) - return true; - - for (final AlternateHandle alt : myAlternates()) { - if (alt.db.hasObjectImpl1(objectId)) - return true; - } - - return tryAgain1() && hasObject1(objectId); - } - - final boolean hasObjectImpl2(final String objectId) { - if (hasObject2(objectId)) - return true; - - for (final AlternateHandle alt : myAlternates()) { - if (alt.db.hasObjectImpl2(objectId)) - return true; - } - - return false; - } - abstract void resolve(Set matches, AbbreviatedObjectId id) throws IOException; @@ -138,180 +83,26 @@ abstract void resolve(Set matches, AbbreviatedObjectId id) abstract Set getShallowCommits() throws IOException; - /** - * Open an object from this database. - *

- * Alternates (if present) are searched automatically. - * - * @param curs - * temporary working space associated with the calling thread. - * @param objectId - * identity of the object to open. - * @return a {@link ObjectLoader} for accessing the data of the named - * object, or null if the object does not exist. - * @throws IOException - */ - ObjectLoader openObject(final WindowCursor curs, final AnyObjectId objectId) - throws IOException { - ObjectLoader ldr; - - ldr = openObjectImpl1(curs, objectId); - if (ldr != null) - return ldr; - - ldr = openObjectImpl2(curs, objectId.name(), objectId); - if (ldr != null) - return ldr; - - return null; - } - - final ObjectLoader openObjectImpl1(final WindowCursor curs, - final AnyObjectId objectId) throws IOException { - ObjectLoader ldr; - - ldr = openObject1(curs, objectId); - if (ldr != null) - return ldr; - - for (final AlternateHandle alt : myAlternates()) { - ldr = alt.db.openObjectImpl1(curs, objectId); - if (ldr != null) - return ldr; - } - - if (tryAgain1()) { - ldr = openObject1(curs, objectId); - if (ldr != null) - return ldr; - } - - return null; - } - - final ObjectLoader openObjectImpl2(final WindowCursor curs, - final String objectName, final AnyObjectId objectId) - throws IOException { - ObjectLoader ldr; - - ldr = openObject2(curs, objectName, objectId); - if (ldr != null) - return ldr; - - for (final AlternateHandle alt : myAlternates()) { - ldr = alt.db.openObjectImpl2(curs, objectName, objectId); - if (ldr != null) - return ldr; - } - - return null; - } - - long getObjectSize(WindowCursor curs, AnyObjectId objectId) - throws IOException { - long sz = getObjectSizeImpl1(curs, objectId); - if (0 <= sz) - return sz; - return getObjectSizeImpl2(curs, objectId.name(), objectId); - } - - final long getObjectSizeImpl1(final WindowCursor curs, - final AnyObjectId objectId) throws IOException { - long sz; - - sz = getObjectSize1(curs, objectId); - if (0 <= sz) - return sz; - - for (final AlternateHandle alt : myAlternates()) { - sz = alt.db.getObjectSizeImpl1(curs, objectId); - if (0 <= sz) - return sz; - } - - if (tryAgain1()) { - sz = getObjectSize1(curs, objectId); - if (0 <= sz) - return sz; - } - - return -1; - } - - final long getObjectSizeImpl2(final WindowCursor curs, - final String objectName, final AnyObjectId objectId) - throws IOException { - long sz; - - sz = getObjectSize2(curs, objectName, objectId); - if (0 <= sz) - return sz; - - for (final AlternateHandle alt : myAlternates()) { - sz = alt.db.getObjectSizeImpl2(curs, objectName, objectId); - if (0 <= sz) - return sz; - } - - return -1; - } - abstract void selectObjectRepresentation(PackWriter packer, ObjectToPack otp, WindowCursor curs) throws IOException; abstract File getDirectory(); - abstract AlternateHandle[] myAlternates(); + abstract File fileFor(AnyObjectId id); - abstract boolean tryAgain1(); - - abstract boolean hasObject1(AnyObjectId objectId); - - abstract boolean hasObject2(String objectId); - - abstract ObjectLoader openObject1(WindowCursor curs, AnyObjectId objectId) + abstract ObjectLoader openObject(WindowCursor curs, AnyObjectId objectId) throws IOException; - abstract ObjectLoader openObject2(WindowCursor curs, String objectName, - AnyObjectId objectId) throws IOException; - - abstract long getObjectSize1(WindowCursor curs, AnyObjectId objectId) + abstract long getObjectSize(WindowCursor curs, AnyObjectId objectId) throws IOException; - abstract long getObjectSize2(WindowCursor curs, String objectName, - AnyObjectId objectId) throws IOException; + abstract ObjectLoader openLooseObject(WindowCursor curs, AnyObjectId id) + throws IOException; abstract InsertLooseObjectResult insertUnpackedObject(File tmp, ObjectId id, boolean createDuplicate) throws IOException; abstract PackFile openPack(File pack) throws IOException; - abstract FileObjectDatabase newCachedFileObjectDatabase(); - abstract Collection getPacks(); - - static class AlternateHandle { - final FileObjectDatabase db; - - AlternateHandle(FileObjectDatabase db) { - this.db = db; - } - - void close() { - db.close(); - } - } - - static class AlternateRepository extends AlternateHandle { - final FileRepository repository; - - AlternateRepository(FileRepository r) { - super(r.getObjectDatabase()); - repository = r; - } - - void close() { - repository.close(); - } - } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java index 33026aad7..148781dfe 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileRepository.java @@ -59,8 +59,8 @@ import org.eclipse.jgit.events.ConfigChangedListener; import org.eclipse.jgit.events.IndexChangedEvent; import org.eclipse.jgit.internal.JGitText; -import org.eclipse.jgit.internal.storage.file.FileObjectDatabase.AlternateHandle; -import org.eclipse.jgit.internal.storage.file.FileObjectDatabase.AlternateRepository; +import org.eclipse.jgit.internal.storage.file.ObjectDirectory.AlternateHandle; +import org.eclipse.jgit.internal.storage.file.ObjectDirectory.AlternateRepository; import org.eclipse.jgit.lib.BaseRepositoryBuilder; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; @@ -368,7 +368,7 @@ public FileBasedConfig getConfig() { */ public Set getAdditionalHaves() { HashSet r = new HashSet(); - for (AlternateHandle d : objectDatabase. myAlternates()) { + for (AlternateHandle d : objectDatabase.myAlternates()) { if (d instanceof AlternateRepository) { Repository repo; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LargePackedDeltaObject.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LargePackedDeltaObject.java index f6bbae2b9..18f15b068 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LargePackedDeltaObject.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/LargePackedDeltaObject.java @@ -173,7 +173,7 @@ public ObjectStream openStream() throws MissingObjectException, IOException { // final ObjectId myId = getObjectId(); final WindowCursor wc = new WindowCursor(db); - ObjectLoader ldr = db.openObject2(wc, myId.name(), myId); + ObjectLoader ldr = db.openLooseObject(wc, myId); if (ldr != null) return ldr.openStream(); 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 3b6901bd5..1795683aa 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 @@ -200,31 +200,19 @@ public void close() { unpackedObjectCache.clear(); final PackList packs = packList.get(); - packList.set(NO_PACKS); - for (final PackFile p : packs.packs) - p.close(); + if (packs != NO_PACKS && packList.compareAndSet(packs, NO_PACKS)) { + for (PackFile p : packs.packs) + p.close(); + } // Fully close all loaded alternates and clear the alternate list. AlternateHandle[] alt = alternates.get(); - if (alt != null) { - alternates.set(null); + if (alt != null && alternates.compareAndSet(alt, null)) { for(final AlternateHandle od : alt) od.close(); } } - /** - * Compute the location of a loose object file. - * - * @param objectId - * identity of the loose object to map to the directory. - * @return location of the object, if it were to exist as a loose object. - */ - @Override - public File fileFor(final AnyObjectId objectId) { - return super.fileFor(objectId); - } - /** * @return unmodifiable collection of all known pack files local to this * directory. Most recent packs are presented first. Packs most @@ -277,37 +265,64 @@ public PackFile openPack(final File pack) @Override public String toString() { - return "ObjectDirectory[" + getDirectory() + "]"; //$NON-NLS-1$ + return "ObjectDirectory[" + getDirectory() + "]"; //$NON-NLS-1$ //$NON-NLS-2$ } - boolean hasObject1(final AnyObjectId objectId) { - if (unpackedObjectCache.isUnpacked(objectId)) + @Override + public boolean has(AnyObjectId objectId) { + return unpackedObjectCache.isUnpacked(objectId) + || hasPackedInSelfOrAlternate(objectId) + || hasLooseInSelfOrAlternate(objectId); + } + + private boolean hasPackedInSelfOrAlternate(AnyObjectId objectId) { + if (hasPackedObject(objectId)) return true; - for (final PackFile p : packList.get().packs) { - try { - if (p.hasObject(objectId)) { - return true; - } - } catch (IOException e) { - // The hasObject call should have only touched the index, - // so any failure here indicates the index is unreadable - // by this process, and the pack is likewise not readable. - // - removePack(p); - continue; - } + for (AlternateHandle alt : myAlternates()) { + if (alt.db.hasPackedInSelfOrAlternate(objectId)) + return true; } return false; } + private boolean hasLooseInSelfOrAlternate(AnyObjectId objectId) { + if (fileFor(objectId).exists()) + return true; + for (AlternateHandle alt : myAlternates()) { + if (alt.db.hasLooseInSelfOrAlternate(objectId)) + return true; + } + return false; + } + + boolean hasPackedObject(AnyObjectId objectId) { + PackList pList; + do { + pList = packList.get(); + for (PackFile p : pList.packs) { + try { + if (p.hasObject(objectId)) + return true; + } catch (IOException e) { + // The hasObject call should have only touched the index, + // so any failure here indicates the index is unreadable + // by this process, and the pack is likewise not readable. + removePack(p); + } + } + } while (searchPacksAgain(pList)); + return false; + } + + @Override void resolve(Set matches, AbbreviatedObjectId id) throws IOException { // Go through the packs once. If we didn't find any resolutions // scan for new packs and check once more. - // int oldSize = matches.size(); - PackList pList = packList.get(); - for (;;) { + PackList pList; + do { + pList = packList.get(); for (PackFile p : pList.packs) { try { p.resolve(matches, id, RESOLVE_ABBREV_LIMIT); @@ -319,15 +334,7 @@ void resolve(Set matches, AbbreviatedObjectId id) if (matches.size() > RESOLVE_ABBREV_LIMIT) return; } - if (matches.size() == oldSize) { - PackList nList = scanPacks(pList); - if (nList == pList || nList.packs.length == 0) - break; - pList = nList; - continue; - } - break; - } + } while (matches.size() == oldSize && searchPacksAgain(pList)); String fanOut = id.name().substring(0, 2); String[] entries = new File(getDirectory(), fanOut).list(); @@ -354,74 +361,164 @@ void resolve(Set matches, AbbreviatedObjectId id) } } - ObjectLoader openObject1(final WindowCursor curs, - final AnyObjectId objectId) throws IOException { + @Override + ObjectLoader openObject(WindowCursor curs, AnyObjectId objectId) + throws IOException { if (unpackedObjectCache.isUnpacked(objectId)) { - ObjectLoader ldr = openObject2(curs, objectId.name(), objectId); + ObjectLoader ldr = openLooseObject(curs, objectId); if (ldr != null) return ldr; - else - unpackedObjectCache.remove(objectId); - } - - PackList pList = packList.get(); - SEARCH: for (;;) { - for (final PackFile p : pList.packs) { - 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); - } - } - return null; } + ObjectLoader ldr = openPackedFromSelfOrAlternate(curs, objectId); + if (ldr != null) + return ldr; + return openLooseFromSelfOrAlternate(curs, objectId); } - long getObjectSize1(final WindowCursor curs, final AnyObjectId objectId) - throws IOException { - PackList pList = packList.get(); - SEARCH: for (;;) { - for (final PackFile p : pList.packs) { - try { - long sz = p.getObjectSize(curs, objectId); - if (0 <= sz) - return sz; - } 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); - } - } - return -1; + private ObjectLoader openPackedFromSelfOrAlternate(WindowCursor curs, + AnyObjectId objectId) { + ObjectLoader ldr = openPackedObject(curs, objectId); + if (ldr != null) + return ldr; + for (AlternateHandle alt : myAlternates()) { + ldr = alt.db.openPackedFromSelfOrAlternate(curs, objectId); + if (ldr != null) + return ldr; } + return null; } - @Override - long getObjectSize2(WindowCursor curs, String objectName, + private ObjectLoader openLooseFromSelfOrAlternate(WindowCursor curs, AnyObjectId objectId) throws IOException { + ObjectLoader ldr = openLooseObject(curs, objectId); + if (ldr != null) + return ldr; + for (AlternateHandle alt : myAlternates()) { + ldr = alt.db.openLooseFromSelfOrAlternate(curs, objectId); + if (ldr != null) + return ldr; + } + return null; + } + + ObjectLoader openPackedObject(WindowCursor curs, AnyObjectId objectId) { + PackList pList; + do { + SEARCH: for (;;) { + pList = packList.get(); + for (PackFile p : pList.packs) { + try { + ObjectLoader ldr = p.get(curs, objectId); + if (ldr != null) + return ldr; + } catch (PackMismatchException e) { + // Pack was modified; refresh the entire pack list. + if (searchPacksAgain(pList)) + continue SEARCH; + } catch (IOException e) { + // Assume the pack is corrupted. + removePack(p); + } + } + break SEARCH; + } + } while (searchPacksAgain(pList)); + return null; + } + + ObjectLoader openLooseObject(WindowCursor curs, AnyObjectId id) + throws IOException { try { - File path = fileFor(objectName); + File path = fileFor(id); FileInputStream in = new FileInputStream(path); try { - return UnpackedObject.getSize(in, objectId, curs); + unpackedObjectCache.add(id); + return UnpackedObject.open(in, path, id, curs); } finally { in.close(); } } catch (FileNotFoundException noFile) { + unpackedObjectCache.remove(id); + return null; + } + } + + long getObjectSize(WindowCursor curs, AnyObjectId id) + throws IOException { + if (unpackedObjectCache.isUnpacked(id)) { + long len = getLooseObjectSize(curs, id); + if (0 <= len) + return len; + } + long len = getPackedSizeFromSelfOrAlternate(curs, id); + if (0 <= len) + return len; + return getLooseSizeFromSelfOrAlternate(curs, id); + } + + private long getPackedSizeFromSelfOrAlternate(WindowCursor curs, + AnyObjectId id) { + long len = getPackedObjectSize(curs, id); + if (0 <= len) + return len; + for (AlternateHandle alt : myAlternates()) { + len = alt.db.getPackedSizeFromSelfOrAlternate(curs, id); + if (0 <= len) + return len; + } + return -1; + } + + private long getLooseSizeFromSelfOrAlternate(WindowCursor curs, + AnyObjectId id) throws IOException { + long len = getLooseObjectSize(curs, id); + if (0 <= len) + return len; + for (AlternateHandle alt : myAlternates()) { + len = alt.db.getLooseSizeFromSelfOrAlternate(curs, id); + if (0 <= len) + return len; + } + return -1; + } + + private long getPackedObjectSize(WindowCursor curs, AnyObjectId id) { + PackList pList; + do { + SEARCH: for (;;) { + pList = packList.get(); + for (PackFile p : pList.packs) { + try { + long len = p.getObjectSize(curs, id); + if (0 <= len) + return len; + } catch (PackMismatchException e) { + // Pack was modified; refresh the entire pack list. + if (searchPacksAgain(pList)) + continue SEARCH; + } catch (IOException e) { + // Assume the pack is corrupted. + removePack(p); + } + } + break SEARCH; + } + } while (searchPacksAgain(pList)); + return -1; + } + + private long getLooseObjectSize(WindowCursor curs, AnyObjectId id) + throws IOException { + try { + FileInputStream in = new FileInputStream(fileFor(id)); + try { + unpackedObjectCache.add(id); + return UnpackedObject.getSize(in, id, curs); + } finally { + in.close(); + } + } catch (FileNotFoundException noFile) { + unpackedObjectCache.remove(id); return -1; } } @@ -454,28 +551,6 @@ void selectObjectRepresentation(PackWriter packer, ObjectToPack otp, h.db.selectObjectRepresentation(packer, otp, curs); } - boolean hasObject2(final String objectName) { - return fileFor(objectName).exists(); - } - - ObjectLoader openObject2(final WindowCursor curs, - final String objectName, final AnyObjectId objectId) - throws IOException { - try { - File path = fileFor(objectName); - FileInputStream in = new FileInputStream(path); - try { - unpackedObjectCache.add(objectId); - return UnpackedObject.open(in, path, objectId, curs); - } finally { - in.close(); - } - } catch (FileNotFoundException noFile) { - unpackedObjectCache.remove(objectId); - return null; - } - } - @Override InsertLooseObjectResult insertUnpackedObject(File tmp, ObjectId id, boolean createDuplicate) throws IOException { @@ -530,11 +605,8 @@ InsertLooseObjectResult insertUnpackedObject(File tmp, ObjectId id, return InsertLooseObjectResult.FAILURE; } - boolean tryAgain1() { - final PackList old = packList.get(); - if (old.snapshot.isModified(packDirectory)) - return old != scanPacks(old); - return false; + private boolean searchPacksAgain(PackList old) { + return old.snapshot.isModified(packDirectory) && old != scanPacks(old); } Config getConfig() { @@ -794,6 +866,20 @@ private AlternateHandle openAlternate(File objdir) throws IOException { return new AlternateHandle(db); } + /** + * Compute the location of a loose object file. + * + * @param objectId + * identity of the loose object to map to the directory. + * @return location of the object, if it were to exist as a loose object. + */ + public File fileFor(AnyObjectId objectId) { + String n = objectId.name(); + String d = n.substring(0, 2); + String f = n.substring(2); + return new File(new File(getDirectory(), d), f); + } + private static final class PackList { /** State just before reading the pack directory. */ final FileSnapshot snapshot; @@ -807,12 +893,37 @@ private static final class PackList { } } + static class AlternateHandle { + final ObjectDirectory db; + + AlternateHandle(ObjectDirectory db) { + this.db = db; + } + + void close() { + db.close(); + } + } + + static class AlternateRepository extends AlternateHandle { + final FileRepository repository; + + AlternateRepository(FileRepository r) { + super(r.getObjectDatabase()); + repository = r; + } + + void close() { + repository.close(); + } + } + @Override public ObjectDatabase newCachedDatabase() { return newCachedFileObjectDatabase(); } - FileObjectDatabase newCachedFileObjectDatabase() { + CachedObjectDirectory newCachedFileObjectDatabase() { return new CachedObjectDirectory(this); } }