{"thread":{"id":"19014","subject":"[JGIT PATCH 1/5] Remove throws IOException from UnpackedObjectLoader.getCachedBytes","startedAt":"2009-04-23T03:36:18Z","lastAt":"2009-04-23T03:36:23Z","messageCount":6,"participants":["Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"112036","messageId":"1240457783-21434-1-git-send-email-spearce@spearce.org","threadId":"19014","inReplyTo":null,"subject":"[JGIT PATCH 0/5] Make ObjectLoader safer during repack","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-23T03:36:18Z","receivedAt":"2009-04-23T03:36:18Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"This is a start to fixing the ObjectLoader save during repack\nproblems I talked about yesterday.  It applies on top of the\nWindowedFile+PackFile patch I sent earlier today.\n\nPatch 1 and 2 are trivial.\n\nPatch 3 fixes the broken 10/10 test case I raised yesterday.\n\nPatch 4 introduces that test case.\n\nPatch 5 starts to fix PackWriter to be safe during repack.\nIts still a bit iffy.  The basic logic here is sound, but I'm more\nworried about someone else calling PackFile.close() and closing the\nunderlying RandomFile while we are otherwise accessing the file\nduring the copyRaw in PackWriter.  I *think* its impossible for\nthat to occur, but I haven't proven it to myself yet, and its late,\nso this one is RFC for now.\n\nShawn O. Pearce (5):\n  Remove throws IOException from UnpackedObjectLoader.getCachedBytes\n  Add missing @Override annotations to UnpackedObjectLoader\n  Fully materialize an ObjectLoader before returning it from\n    ObjectDatabase\n  Test that ObjectLoader stays valid across repacks\n  Teach PackWriter to recover from removed/replaced packs\n\n .../org/spearce/jgit/lib/ConcurrentRepackTest.java |   40 +++++++\n .../jgit/lib/DeltaOfsPackedObjectLoader.java       |    7 +-\n .../spearce/jgit/lib/DeltaPackedObjectLoader.java  |   41 +++----\n .../jgit/lib/DeltaRefPackedObjectLoader.java       |    9 +-\n .../src/org/spearce/jgit/lib/ObjectDirectory.java  |    3 +-\n .../src/org/spearce/jgit/lib/ObjectLoader.java     |   23 +---\n .../src/org/spearce/jgit/lib/PackFile.java         |   14 +-\n .../src/org/spearce/jgit/lib/PackWriter.java       |  118 +++++++++++++-------\n .../org/spearce/jgit/lib/PackedObjectLoader.java   |   78 +++++++++++--\n .../org/spearce/jgit/lib/UnpackedObjectLoader.java |    4 +-\n .../spearce/jgit/lib/WholePackedObjectLoader.java  |   21 ++--\n .../src/org/spearce/jgit/lib/WindowCache.java      |   59 +++++++----\n 12 files changed, 280 insertions(+), 137 deletions(-)\n"},{"id":"112033","messageId":"1240457783-21434-2-git-send-email-spearce@spearce.org","threadId":"19014","inReplyTo":"1240457783-21434-1-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 1/5] Remove throws IOException from UnpackedObjectLoader.getCachedBytes","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-23T03:36:19Z","receivedAt":"2009-04-23T03:36:19Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"The UnpackedObjectLoader always fully materializes itself during the\nobject's constructor.  Thus the cached bytes are always ready on any\ninvocation of getCachedBytes(), and this method will never fail.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../org/spearce/jgit/lib/UnpackedObjectLoader.java |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java\nindex 7552b42..1352b72 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java\n@@ -201,7 +201,7 @@ public long getSize() {\n \t}\n \n \t@Override\n-\tpublic byte[] getCachedBytes() throws IOException {\n+\tpublic byte[] getCachedBytes() {\n \t\treturn bytes;\n \t}\n \n-- \n1.6.3.rc1.205.g37f8\n"},{"id":"112037","messageId":"1240457783-21434-3-git-send-email-spearce@spearce.org","threadId":"19014","inReplyTo":"1240457783-21434-2-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 2/5] Add missing @Override annotations to UnpackedObjectLoader","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-23T03:36:20Z","receivedAt":"2009-04-23T03:36:20Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"These are declared by the base class, ObjectLoader.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../org/spearce/jgit/lib/UnpackedObjectLoader.java |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java\nindex 1352b72..e2de41c 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java\n@@ -192,10 +192,12 @@ private void decompress(final AnyObjectId id, final Inflater inf, int p)\n \t\t\tthrow new CorruptObjectException(id, \"incorrect length\");\n \t}\n \n+\t@Override\n \tpublic int getType() {\n \t\treturn objectType;\n \t}\n \n+\t@Override\n \tpublic long getSize() {\n \t\treturn objectSize;\n \t}\n-- \n1.6.3.rc1.205.g37f8\n"},{"id":"112034","messageId":"1240457783-21434-4-git-send-email-spearce@spearce.org","threadId":"19014","inReplyTo":"1240457783-21434-3-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 3/5] Fully materialize an ObjectLoader before returning it from ObjectDatabase","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-23T03:36:21Z","receivedAt":"2009-04-23T03:36:21Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"By forcing the ObjectLoader to fully reconstruct the requested object\nand store it under getCachedBytes() before we return the ObjectLoader\nto the caller we can ensure that the underlying PackFile remains valid\nthroughout the entire load.\n\nIf memory pressure causes the PackFile's windows to be evicted, and\nthe pack closes, and is replaced underneath of us, we can correctly\ncatch and handle the PackMismatchException or IOException by closing\nthe pack, rescanning the ObjectDirectory, and starting our search all\nover again.  Doing so is expensive, as we may have already unpacked\na large part of the object's delta chain before finding the pack was\nmodified, but its better than failing outright due to a race condition.\n\nNote that this does not resolve the concurrent access problems in\nPackWriter, as the cached PackedObjectLoaders are referenced very\nlate, and go back to the original PackFile, not the cached store.\n\nBecause we fully access the pack data up front during materialize()\nwe no longer need to hold the WindowCursor inside of the individual\nloader instance.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../jgit/lib/DeltaOfsPackedObjectLoader.java       |    7 ++-\n .../spearce/jgit/lib/DeltaPackedObjectLoader.java  |   41 +++++++---------\n .../jgit/lib/DeltaRefPackedObjectLoader.java       |    9 ++--\n .../src/org/spearce/jgit/lib/ObjectDirectory.java  |    3 +-\n .../src/org/spearce/jgit/lib/ObjectLoader.java     |   23 +++-------\n .../src/org/spearce/jgit/lib/PackFile.java         |   14 +++---\n .../src/org/spearce/jgit/lib/PackWriter.java       |    4 +-\n .../org/spearce/jgit/lib/PackedObjectLoader.java   |   48 +++++++++++++++----\n .../spearce/jgit/lib/WholePackedObjectLoader.java  |   21 +++++----\n 9 files changed, 94 insertions(+), 76 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaOfsPackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaOfsPackedObjectLoader.java\nindex 5c9fb00..ee2a0de 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaOfsPackedObjectLoader.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaOfsPackedObjectLoader.java\n@@ -46,14 +46,15 @@\n class DeltaOfsPackedObjectLoader extends DeltaPackedObjectLoader {\n \tprivate final long deltaBase;\n \n-\tDeltaOfsPackedObjectLoader(final WindowCursor curs, final PackFile pr,\n+\tDeltaOfsPackedObjectLoader(final PackFile pr,\n \t\t\tfinal long dataOffset, final long objectOffset, final int deltaSz,\n \t\t\tfinal long base) {\n-\t\tsuper(curs, pr, dataOffset, objectOffset, deltaSz);\n+\t\tsuper(pr, dataOffset, objectOffset, deltaSz);\n \t\tdeltaBase = base;\n \t}\n \n-\tprotected PackedObjectLoader getBaseLoader() throws IOException {\n+\tprotected PackedObjectLoader getBaseLoader(final WindowCursor curs)\n+\t\t\tthrows IOException {\n \t\treturn pack.resolveBase(curs, deltaBase);\n \t}\n \ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaPackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaPackedObjectLoader.java\nindex 85f2bda..8194d94 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaPackedObjectLoader.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaPackedObjectLoader.java\n@@ -49,47 +49,39 @@\n \n \tprivate final int deltaSize;\n \n-\tDeltaPackedObjectLoader(final WindowCursor curs, final PackFile pr,\n-\t\t\tfinal long dataOffset, final long objectOffset, final int deltaSz) {\n-\t\tsuper(curs, pr, dataOffset, objectOffset);\n+\tDeltaPackedObjectLoader(final PackFile pr, final long dataOffset,\n+\t\t\tfinal long objectOffset, final int deltaSz) {\n+\t\tsuper(pr, dataOffset, objectOffset);\n \t\tobjectType = -1;\n \t\tdeltaSize = deltaSz;\n \t}\n \n-\tpublic int getType() throws IOException {\n-\t\tif (objectType < 0)\n-\t\t\tgetCachedBytes();\n-\t\treturn objectType;\n-\t}\n-\n-\tpublic long getSize() throws IOException {\n-\t\tif (objectType < 0)\n-\t\t\tgetCachedBytes();\n-\t\treturn objectSize;\n-\t}\n-\n \t@Override\n-\tpublic byte[] getCachedBytes() throws IOException {\n+\tpublic void materialize(final WindowCursor curs) throws IOException {\n+\t\tif (cachedBytes != null) {\n+\t\t\treturn;\n+\t\t}\n+\n \t\tif (objectType != OBJ_COMMIT) {\n \t\t\tfinal UnpackedObjectCache.Entry cache = pack.readCache(dataOffset);\n \t\t\tif (cache != null) {\n \t\t\t\tcurs.release();\n \t\t\t\tobjectType = cache.type;\n \t\t\t\tobjectSize = cache.data.length;\n-\t\t\t\treturn cache.data;\n+\t\t\t\tcachedBytes = cache.data;\n \t\t\t}\n \t\t}\n \n \t\ttry {\n-\t\t\tfinal PackedObjectLoader baseLoader = getBaseLoader();\n-\t\t\tfinal byte[] data = BinaryDelta.apply(baseLoader.getCachedBytes(),\n+\t\t\tfinal PackedObjectLoader baseLoader = getBaseLoader(curs);\n+\t\t\tbaseLoader.materialize(curs);\n+\t\t\tcachedBytes = BinaryDelta.apply(baseLoader.getCachedBytes(),\n \t\t\t\t\tpack.decompress(dataOffset, deltaSize, curs));\n \t\t\tcurs.release();\n \t\t\tobjectType = baseLoader.getType();\n-\t\t\tobjectSize = data.length;\n+\t\t\tobjectSize = cachedBytes.length;\n \t\t\tif (objectType != OBJ_COMMIT)\n-\t\t\t\tpack.saveCache(dataOffset, data, objectType);\n-\t\t\treturn data;\n+\t\t\t\tpack.saveCache(dataOffset, cachedBytes, objectType);\n \t\t} catch (DataFormatException dfe) {\n \t\t\tfinal CorruptObjectException coe;\n \t\t\tcoe = new CorruptObjectException(\"Object at \" + dataOffset + \" in \"\n@@ -105,8 +97,11 @@ public long getRawSize() {\n \t}\n \n \t/**\n+\t * @param curs\n+\t *            temporary thread storage during data access.\n \t * @return the object loader for the base object\n \t * @throws IOException\n \t */\n-\tprotected abstract PackedObjectLoader getBaseLoader() throws IOException;\n+\tprotected abstract PackedObjectLoader getBaseLoader(WindowCursor curs)\n+\t\t\tthrows IOException;\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaRefPackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaRefPackedObjectLoader.java\nindex b126bbd..82a3a7f 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaRefPackedObjectLoader.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaRefPackedObjectLoader.java\n@@ -46,14 +46,15 @@\n class DeltaRefPackedObjectLoader extends DeltaPackedObjectLoader {\n \tprivate final ObjectId deltaBase;\n \n-\tDeltaRefPackedObjectLoader(final WindowCursor curs, final PackFile pr,\n+\tDeltaRefPackedObjectLoader(final PackFile pr,\n \t\t\tfinal long dataOffset, final long objectOffset, final int deltaSz,\n \t\t\tfinal ObjectId base) {\n-\t\tsuper(curs, pr, dataOffset, objectOffset, deltaSz);\n+\t\tsuper(pr, dataOffset, objectOffset, deltaSz);\n \t\tdeltaBase = base;\n \t}\n \n-\tprotected PackedObjectLoader getBaseLoader() throws IOException {\n+\tprotected PackedObjectLoader getBaseLoader(final WindowCursor curs)\n+\t\t\tthrows IOException {\n \t\tfinal PackedObjectLoader or = pack.get(curs, deltaBase);\n \t\tif (or == null)\n \t\t\tthrow new MissingObjectException(deltaBase, \"delta base\");\n@@ -61,7 +62,7 @@ protected PackedObjectLoader getBaseLoader() throws IOException {\n \t}\n \n \t@Override\n-\tpublic int getRawType() throws IOException {\n+\tpublic int getRawType() {\n \t\treturn Constants.OBJ_REF_DELTA;\n \t}\n \ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\nindex 6ba0180..3e9ed1c 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java\n@@ -197,8 +197,9 @@ protected ObjectLoader openObject1(final WindowCursor curs,\n \t\tSEARCH: for (;;) {\n \t\t\tfor (final PackFile p : pList) {\n \t\t\t\ttry {\n-\t\t\t\t\tfinal ObjectLoader ldr = p.get(curs, objectId);\n+\t\t\t\t\tfinal PackedObjectLoader ldr = p.get(curs, objectId);\n \t\t\t\t\tif (ldr != null) {\n+\t\t\t\t\t\tldr.materialize(curs);\n \t\t\t\t\t\treturn ldr;\n \t\t\t\t\t}\n \t\t\t\t} catch (PackMismatchException e) {\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java\nindex 5485d8d..6b92694 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java\n@@ -38,7 +38,6 @@\n \n package org.spearce.jgit.lib;\n \n-import java.io.IOException;\n \n /**\n  * Base class for a set of loaders for different representations of Git objects.\n@@ -47,15 +46,13 @@\n public abstract class ObjectLoader {\n \t/**\n \t * @return Git in pack object type, see {@link Constants}.\n-\t * @throws IOException\n \t */\n-\tpublic abstract int getType() throws IOException;\n+\tpublic abstract int getType();\n \n \t/**\n \t * @return size of object in bytes\n-\t * @throws IOException\n \t */\n-\tpublic abstract long getSize() throws IOException;\n+\tpublic abstract long getSize();\n \n \t/**\n \t * Obtain a copy of the bytes of this object.\n@@ -64,10 +61,8 @@\n \t * be modified by the caller.\n \t * \n \t * @return the bytes of this object.\n-\t * @throws IOException\n-\t *             the object cannot be read.\n \t */\n-\tpublic final byte[] getBytes() throws IOException {\n+\tpublic final byte[] getBytes() {\n \t\tfinal byte[] data = getCachedBytes();\n \t\tfinal byte[] copy = new byte[data.length];\n \t\tSystem.arraycopy(data, 0, copy, 0, data.length);\n@@ -83,25 +78,19 @@\n \t * Changes (if made) will affect the cache but not the repository itself.\n \t * \n \t * @return the cached bytes of this object. Do not modify it.\n-\t * @throws IOException\n-\t *             the object cannot be read.\n \t */\n-\tpublic abstract byte[] getCachedBytes() throws IOException;\n+\tpublic abstract byte[] getCachedBytes();\n \n \t/**\n \t * @return raw object type from object header, as stored in storage (pack,\n \t *         loose file). This may be different from {@link #getType()} result\n \t *         for packs (see {@link Constants}).\n-\t * @throws IOException\n-\t *             when type cannot be read from the object header.\n \t */\n-\tpublic abstract int getRawType() throws IOException;\n+\tpublic abstract int getRawType();\n \n \t/**\n \t * @return raw size of object from object header (pack, loose file).\n \t *         Interpretation of this value depends on {@link #getRawType()}.\n-\t * @throws IOException\n-\t *             when raw size cannot be read from the object header.\n \t */\n-\tpublic abstract long getRawSize() throws IOException;\n+\tpublic abstract long getRawSize();\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\nindex dccc067..813ebc7 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java\n@@ -244,11 +244,11 @@ final void saveCache(final long position, final byte[] data, final int type) {\n \t}\n \n \tfinal void copyRawData(final PackedObjectLoader loader,\n-\t\t\tfinal OutputStream out, final byte buf[]) throws IOException {\n+\t\t\tfinal OutputStream out, final byte buf[], final WindowCursor curs)\n+\t\t\tthrows IOException {\n \t\tfinal long objectOffset = loader.objectOffset;\n \t\tfinal long dataOffset = loader.dataOffset;\n \t\tfinal int cnt = (int) (findEndOffset(objectOffset) - dataOffset);\n-\t\tfinal WindowCursor curs = loader.curs;\n \t\tfinal PackIndex idx = idx();\n \n \t\tif (idx.hasCRC32Support()) {\n@@ -436,8 +436,8 @@ private PackedObjectLoader reader(final WindowCursor curs,\n \t\tcase Constants.OBJ_TREE:\n \t\tcase Constants.OBJ_BLOB:\n \t\tcase Constants.OBJ_TAG:\n-\t\t\treturn new WholePackedObjectLoader(curs, this, pos, objOffset,\n-\t\t\t\t\ttypeCode, (int) dataSize);\n+\t\t\treturn new WholePackedObjectLoader(this, pos, objOffset, typeCode,\n+\t\t\t\t\t(int) dataSize);\n \n \t\tcase Constants.OBJ_OFS_DELTA: {\n \t\t\treadFully(pos, ib, 0, 20, curs);\n@@ -450,12 +450,12 @@ private PackedObjectLoader reader(final WindowCursor curs,\n \t\t\t\tofs <<= 7;\n \t\t\t\tofs += (c & 127);\n \t\t\t}\n-\t\t\treturn new DeltaOfsPackedObjectLoader(curs, this, pos + p,\n-\t\t\t\t\tobjOffset, (int) dataSize, objOffset - ofs);\n+\t\t\treturn new DeltaOfsPackedObjectLoader(this, pos + p, objOffset,\n+\t\t\t\t\t(int) dataSize, objOffset - ofs);\n \t\t}\n \t\tcase Constants.OBJ_REF_DELTA: {\n \t\t\treadFully(pos, ib, 0, 20, curs);\n-\t\t\treturn new DeltaRefPackedObjectLoader(curs, this, pos + ib.length,\n+\t\t\treturn new DeltaRefPackedObjectLoader(this, pos + ib.length,\n \t\t\t\t\tobjOffset, (int) dataSize, ObjectId.fromRaw(ib));\n \t\t}\n \t\tdefault:\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java\nindex c2e6c4b..e1397fd 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java\n@@ -720,7 +720,7 @@ private void writeWholeObject(final ObjectToPack otp) throws IOException {\n \t\tif (otp.hasReuseLoader()) {\n \t\t\tfinal PackedObjectLoader loader = otp.getReuseLoader();\n \t\t\twriteObjectHeader(otp.getType(), loader.getSize());\n-\t\t\tloader.copyRawData(out, buf);\n+\t\t\tloader.copyRawData(out, buf, windowCursor);\n \t\t\totp.disposeLoader();\n \t\t} else {\n \t\t\tfinal ObjectLoader loader = db.openObject(windowCursor, otp);\n@@ -756,7 +756,7 @@ private void writeDeltaObject(final ObjectToPack otp) throws IOException {\n \t\t\totp.getDeltaBaseId().copyRawTo(buf, 0);\n \t\t\tout.write(buf, 0, Constants.OBJECT_ID_LENGTH);\n \t\t}\n-\t\tloader.copyRawData(out, buf);\n+\t\tloader.copyRawData(out, buf, windowCursor);\n \t\totp.disposeLoader();\n \t}\n \ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackedObjectLoader.java\nindex 56985c1..60333e7 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackedObjectLoader.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackedObjectLoader.java\n@@ -47,8 +47,6 @@\n abstract class PackedObjectLoader extends ObjectLoader {\n \tprotected final PackFile pack;\n \n-\tprotected final WindowCursor curs;\n-\n \tprotected final long dataOffset;\n \n \tprotected final long objectOffset;\n@@ -57,26 +55,53 @@\n \n \tprotected int objectSize;\n \n-\tPackedObjectLoader(final WindowCursor c, final PackFile pr,\n-\t\t\tfinal long dataOffset, final long objectOffset) {\n-\t\tcurs = c;\n+\tprotected byte[] cachedBytes;\n+\n+\tPackedObjectLoader(final PackFile pr, final long dataOffset,\n+\t\t\tfinal long objectOffset) {\n \t\tpack = pr;\n \t\tthis.dataOffset = dataOffset;\n \t\tthis.objectOffset = objectOffset;\n \t}\n \n-\tpublic int getType() throws IOException {\n+\t/**\n+\t * Force this object to be loaded into memory and pinned in this loader.\n+\t * <p>\n+\t * Once materialized, subsequent get operations for the following methods\n+\t * will always succeed without raising an exception, as all information is\n+\t * pinned in memory by this loader instance.\n+\t * <ul>\n+\t * <li>{@link #getType()}</li>\n+\t * <li>{@link #getSize()}</li>\n+\t * <li>{@link #getBytes()}, {@link #getCachedBytes}</li>\n+\t * <li>{@link #getRawSize()}</li>\n+\t * <li>{@link #getRawType()}</li>\n+\t * </ul>\n+\t *\n+\t * @param curs\n+\t *            temporary thread storage during data access.\n+\t * @throws IOException\n+\t *             the object cannot be read.\n+\t */\n+\tpublic abstract void materialize(WindowCursor curs) throws IOException;\n+\n+\tpublic final int getType() {\n \t\treturn objectType;\n \t}\n \n-\tpublic long getSize() throws IOException {\n+\tpublic final long getSize() {\n \t\treturn objectSize;\n \t}\n \n+\t@Override\n+\tpublic final byte[] getCachedBytes() {\n+\t\treturn cachedBytes;\n+\t}\n+\n \t/**\n \t * @return offset of object data within pack file\n \t */\n-\tpublic long getDataOffset() {\n+\tpublic final long getDataOffset() {\n \t\treturn dataOffset;\n \t}\n \n@@ -92,11 +117,14 @@ public long getDataOffset() {\n \t * @param buf\n \t *            temporary buffer used during copying. Recommended size is at\n \t *            least few kB.\n+\t * @param curs\n+\t *            temporary thread storage during data access.\n \t * @throws IOException\n \t *             when the object cannot be read.\n \t */\n-\tpublic void copyRawData(OutputStream out, byte buf[]) throws IOException {\n-\t\tpack.copyRawData(this, out, buf);\n+\tpublic void copyRawData(OutputStream out, byte buf[], WindowCursor curs)\n+\t\t\tthrows IOException {\n+\t\tpack.copyRawData(this, out, buf, curs);\n \t}\n \n \t/**\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WholePackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WholePackedObjectLoader.java\nindex 3b4f90d..a1df3d2 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WholePackedObjectLoader.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WholePackedObjectLoader.java\n@@ -46,30 +46,33 @@\n class WholePackedObjectLoader extends PackedObjectLoader {\n \tprivate static final int OBJ_COMMIT = Constants.OBJ_COMMIT;\n \n-\tWholePackedObjectLoader(final WindowCursor curs, final PackFile pr,\n-\t\t\tfinal long dataOffset, final long objectOffset, final int type,\n-\t\t\tfinal int size) {\n-\t\tsuper(curs, pr, dataOffset, objectOffset);\n+\tWholePackedObjectLoader(final PackFile pr, final long dataOffset,\n+\t\t\tfinal long objectOffset, final int type, final int size) {\n+\t\tsuper(pr, dataOffset, objectOffset);\n \t\tobjectType = type;\n \t\tobjectSize = size;\n \t}\n \n \t@Override\n-\tpublic byte[] getCachedBytes() throws IOException {\n+\tpublic void materialize(final WindowCursor curs) throws IOException {\n+\t\tif (cachedBytes != null) {\n+\t\t\treturn;\n+\t\t}\n+\n \t\tif (objectType != OBJ_COMMIT) {\n \t\t\tfinal UnpackedObjectCache.Entry cache = pack.readCache(dataOffset);\n \t\t\tif (cache != null) {\n \t\t\t\tcurs.release();\n-\t\t\t\treturn cache.data;\n+\t\t\t\tcachedBytes = cache.data;\n+\t\t\t\treturn;\n \t\t\t}\n \t\t}\n \n \t\ttry {\n-\t\t\tfinal byte[] data = pack.decompress(dataOffset, objectSize, curs);\n+\t\t\tcachedBytes = pack.decompress(dataOffset, objectSize, curs);\n \t\t\tcurs.release();\n \t\t\tif (objectType != OBJ_COMMIT)\n-\t\t\t\tpack.saveCache(dataOffset, data, objectType);\n-\t\t\treturn data;\n+\t\t\t\tpack.saveCache(dataOffset, cachedBytes, objectType);\n \t\t} catch (DataFormatException dfe) {\n \t\t\tfinal CorruptObjectException coe;\n \t\t\tcoe = new CorruptObjectException(\"Object at \" + dataOffset + \" in \"\n-- \n1.6.3.rc1.205.g37f8\n"},{"id":"112035","messageId":"1240457783-21434-5-git-send-email-spearce@spearce.org","threadId":"19014","inReplyTo":"1240457783-21434-4-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 4/5] Test that ObjectLoader stays valid across repacks","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-23T03:36:22Z","receivedAt":"2009-04-23T03:36:22Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"What we are trying to verify is that an ObjectLoader remains valid\nif the underlying storage for the object has moved, such as when a\nrepository is repacked, the old pack was deleted, and the object is\nnow in the new pack.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../org/spearce/jgit/lib/ConcurrentRepackTest.java |   40 ++++++++++++++++++++\n 1 files changed, 40 insertions(+), 0 deletions(-)\n\ndiff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\nindex 825fbb8..b56e0f4 100644\n--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\n+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ConcurrentRepackTest.java\n@@ -41,6 +41,7 @@\n import java.io.File;\n import java.io.FileOutputStream;\n import java.io.IOException;\n+import java.util.Arrays;\n \n import org.spearce.jgit.errors.IncorrectObjectTypeException;\n import org.spearce.jgit.errors.MissingObjectException;\n@@ -128,6 +129,45 @@ public void testObjectMovedWithinPack()\n \t\tassertEquals(o2.name(), parse(o2).name());\n \t}\n \n+\tpublic void testObjectMovedToNewPack2()\n+\t\t\tthrows IncorrectObjectTypeException, IOException {\n+\t\t// Create an object and pack it. Then remove that pack and put the\n+\t\t// object into a different pack file, with some other object. We\n+\t\t// still should be able to access the objects.\n+\t\t//\n+\t\tfinal Repository eden = createNewEmptyRepo();\n+\t\tfinal RevObject o1 = writeBlob(eden, \"o1\");\n+\t\tfinal File[] out1 = pack(eden, o1);\n+\t\tassertEquals(o1.name(), parse(o1).name());\n+\n+\t\tfinal ObjectLoader load1 = db.openBlob(o1);\n+\t\tassertNotNull(load1);\n+\n+\t\tfinal RevObject o2 = writeBlob(eden, \"o2\");\n+\t\tpack(eden, o2, o1);\n+\n+\t\t// Force close, and then delete, the old pack.\n+\t\t//\n+\t\twhackCache();\n+\t\tdelete(out1);\n+\n+\t\t// Now here is the interesting thing... can the loader we made\n+\t\t// earlier still resolve the object, even though its underlying\n+\t\t// pack is gone, but the object still exists.\n+\t\t//\n+\t\tfinal ObjectLoader load2 = db.openBlob(o1);\n+\t\tassertNotNull(load2);\n+\t\tassertNotSame(load1, load2);\n+\n+\t\tfinal byte[] data2 = load2.getCachedBytes();\n+\t\tfinal byte[] data1 = load1.getCachedBytes();\n+\t\tassertNotNull(data2);\n+\t\tassertNotNull(data1);\n+\t\tassertNotSame(data1, data2); // cache should be per-pack, not per object\n+\t\tassertTrue(Arrays.equals(data1, data2));\n+\t\tassertEquals(load2.getType(), load1.getType());\n+\t}\n+\n \tprivate static void whackCache() {\n \t\tfinal WindowCacheConfig config = new WindowCacheConfig();\n \n-- \n1.6.3.rc1.205.g37f8\n"},{"id":"112038","messageId":"1240457783-21434-6-git-send-email-spearce@spearce.org","threadId":"19014","inReplyTo":"1240457783-21434-5-git-send-email-spearce@spearce.org","subject":"[JGIT RFC PATCH 5/5] Teach PackWriter to recover from removed/replaced packs","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-04-23T03:36:23Z","receivedAt":"2009-04-23T03:36:23Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"A concurrently running \"git gc\" in the same repository could cause a\nPackFile that was previously identified for object reuse to disappear\n(or be rewritten) between the time that a segment was selected to\nbe reused, and when we actually need to copy the raw data from the\npack to the output stream.\n\nWe now peg the pack file open during the reuse period, ensuring\nthat the underlying file descriptor cannot be closed while we are\ncopying data, even if memory pressure gets high and windows are\nevicted from the WindowCache.\n\nIf the pack file is gone (or has been rewritten) since we originally\npicked it for reuse we throw away that reuse decision and make\nit again.  This is a relative waste of CPU and disk IO, as we have\nto do work twice, but it should be fairly infrequent as repositories\nare not repacked that often.  If we do have to recompute one object,\nit is likely that we may need to recompute all reuse decisions for\nthe remainder of this pack output stream, but doing a bit more work\nand succeeding is better than failing outright with an obtuse error.\n\nThe way we handle the recovery is subject to livelock.  We could\npick a new reuse location, see it disappear before we can get a pin,\nand need to select another one.  Livelock however is not likely\nhere, as the situation can only happen when the selected pack\nfile has been deleted or overwritten.  Repacking takes some time\non any repository, typically longer than a single PackWriter may\nneed to stream to a network client.  It is highly improbable that\nthe repository administator is running \"while true; git gc; done\",\nas that would suck up all system resources and generally make the\nhost unresponsive anyway.  So, long story short, we should be OK\nagainst livelock if the repository administrator isn't hellbent\non otherwise sucking up CPU usage through repeat git gc attempts.\nAnd if they are, we'll just help them out by falling for the obvious\nlivelock case.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n\n As I said in the cover letter of this series; this one is still\n iffy.  I think its sound as-is, but I'm not convinced that this\n patch is sufficient to cover everything that can happen.\n\n .../src/org/spearce/jgit/lib/PackWriter.java       |  118 +++++++++++++-------\n .../org/spearce/jgit/lib/PackedObjectLoader.java   |   30 +++++\n .../src/org/spearce/jgit/lib/WindowCache.java      |   59 +++++++----\n 3 files changed, 145 insertions(+), 62 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java\nindex e1397fd..2da8bbc 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java\n@@ -606,14 +606,7 @@ private void searchForReuse() throws IOException {\n \t\t\t\t\tthrow new IOException(\n \t\t\t\t\t\t\t\"Packing cancelled during objects writing\");\n \t\t\t\treuseLoaders.clear();\n-\t\t\t\tdb.openObjectInAllPacks(otp, reuseLoaders, windowCursor);\n-\t\t\t\tif (reuseDeltas) {\n-\t\t\t\t\tselectDeltaReuseForObject(otp, reuseLoaders);\n-\t\t\t\t}\n-\t\t\t\t// delta reuse is preferred over object reuse\n-\t\t\t\tif (reuseObjects && !otp.hasReuseLoader()) {\n-\t\t\t\t\tselectObjectReuseForObject(otp, reuseLoaders);\n-\t\t\t\t}\n+\t\t\t\tsearchForReuse(reuseLoaders, otp);\n \t\t\t\tinitMonitor.update(1);\n \t\t\t}\n \t\t}\n@@ -621,6 +614,19 @@ private void searchForReuse() throws IOException {\n \t\tinitMonitor.endTask();\n \t}\n \n+\tprivate void searchForReuse(\n+\t\t\tfinal Collection<PackedObjectLoader> reuseLoaders,\n+\t\t\tfinal ObjectToPack otp) throws IOException {\n+\t\tdb.openObjectInAllPacks(otp, reuseLoaders, windowCursor);\n+\t\tif (reuseDeltas) {\n+\t\t\tselectDeltaReuseForObject(otp, reuseLoaders);\n+\t\t}\n+\t\t// delta reuse is preferred over object reuse\n+\t\tif (reuseObjects && !otp.hasReuseLoader()) {\n+\t\t\tselectObjectReuseForObject(otp, reuseLoaders);\n+\t\t}\n+\t}\n+\n \tprivate void selectDeltaReuseForObject(final ObjectToPack otp,\n \t\t\tfinal Collection<PackedObjectLoader> loaders) throws IOException {\n \t\tPackedObjectLoader bestLoader = null;\n@@ -707,40 +713,69 @@ private void writeObject(final ObjectToPack otp) throws IOException {\n \n \t\tout.resetCRC32();\n \t\totp.setOffset(out.length());\n-\t\tif (otp.isDeltaRepresentation())\n-\t\t\twriteDeltaObject(otp);\n-\t\telse\n-\t\t\twriteWholeObject(otp);\n+\t\t\n+\t\tfinal PackedObjectLoader reuse = open(otp);\n+\t\tif (reuse != null) {\n+\t\t\ttry {\n+\t\t\t\tif (otp.isDeltaRepresentation()) {\n+\t\t\t\t\twriteDeltaObjectReuse(otp, reuse);\n+\t\t\t\t} else {\n+\t\t\t\t\twriteObjectHeader(otp.getType(), reuse.getSize());\n+\t\t\t\t\treuse.copyRawData(out, buf, windowCursor);\n+\t\t\t\t}\n+\t\t\t} finally {\n+\t\t\t\treuse.endCopyRawData();\n+\t\t\t}\n+\t\t} else if (otp.isDeltaRepresentation()) {\n+\t\t\tthrow new IOException(\"creating deltas is not implemented\");\n+\t\t} else {\n+\t\t\twriteWholeObjectDeflate(otp);\n+\t\t}\n \t\totp.setCRC(out.getCRC32());\n \n \t\twriteMonitor.update(1);\n \t}\n \n-\tprivate void writeWholeObject(final ObjectToPack otp) throws IOException {\n-\t\tif (otp.hasReuseLoader()) {\n-\t\t\tfinal PackedObjectLoader loader = otp.getReuseLoader();\n-\t\t\twriteObjectHeader(otp.getType(), loader.getSize());\n-\t\t\tloader.copyRawData(out, buf, windowCursor);\n-\t\t\totp.disposeLoader();\n-\t\t} else {\n-\t\t\tfinal ObjectLoader loader = db.openObject(windowCursor, otp);\n-\t\t\tfinal byte[] data = loader.getCachedBytes();\n-\t\t\twriteObjectHeader(otp.getType(), data.length);\n-\t\t\tdeflater.reset();\n-\t\t\tdeflater.setInput(data, 0, data.length);\n-\t\t\tdeflater.finish();\n-\t\t\tdo {\n-\t\t\t\tfinal int n = deflater.deflate(buf, 0, buf.length);\n-\t\t\t\tif (n > 0)\n-\t\t\t\t\tout.write(buf, 0, n);\n-\t\t\t} while (!deflater.finished());\n-\t\t}\n-\t}\n-\n-\tprivate void writeDeltaObject(final ObjectToPack otp) throws IOException {\n-\t\tfinal PackedObjectLoader loader = otp.getReuseLoader();\n+\tprivate PackedObjectLoader open(final ObjectToPack otp) throws IOException {\n+\t\tfor (;;) {\n+\t\t\tPackedObjectLoader reuse = otp.useLoader();\n+\t\t\tif (reuse == null) {\n+\t\t\t\treturn null;\n+\t\t\t}\n+\n+\t\t\ttry {\n+\t\t\t\treuse.beginCopyRawData();\n+\t\t\t\treturn reuse;\n+\t\t\t} catch (IOException err) {\n+\t\t\t\t// The pack we found the object in originally is gone, or\n+\t\t\t\t// it has been overwritten with a different layout.\n+\t\t\t\t//\n+\t\t\t\totp.clearDeltaBase();\n+\t\t\t\tsearchForReuse(new ArrayList<PackedObjectLoader>(), otp);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tprivate void writeWholeObjectDeflate(final ObjectToPack otp)\n+\t\t\tthrows IOException {\n+\t\tfinal ObjectLoader loader = db.openObject(windowCursor, otp);\n+\t\tfinal byte[] data = loader.getCachedBytes();\n+\t\twriteObjectHeader(otp.getType(), data.length);\n+\t\tdeflater.reset();\n+\t\tdeflater.setInput(data, 0, data.length);\n+\t\tdeflater.finish();\n+\t\tdo {\n+\t\t\tfinal int n = deflater.deflate(buf, 0, buf.length);\n+\t\t\tif (n > 0)\n+\t\t\t\tout.write(buf, 0, n);\n+\t\t} while (!deflater.finished());\n+\t}\n+\n+\tprivate void writeDeltaObjectReuse(final ObjectToPack otp,\n+\t\t\tfinal PackedObjectLoader reuse) throws IOException {\n \t\tif (deltaBaseAsOffset && otp.getDeltaBase() != null) {\n-\t\t\twriteObjectHeader(Constants.OBJ_OFS_DELTA, loader.getRawSize());\n+\t\t\twriteObjectHeader(Constants.OBJ_OFS_DELTA, reuse.getRawSize());\n \n \t\t\tfinal ObjectToPack deltaBase = otp.getDeltaBase();\n \t\t\tlong offsetDiff = otp.getOffset() - deltaBase.getOffset();\n@@ -752,12 +787,11 @@ private void writeDeltaObject(final ObjectToPack otp) throws IOException {\n \n \t\t\tout.write(buf, pos, buf.length - pos);\n \t\t} else {\n-\t\t\twriteObjectHeader(Constants.OBJ_REF_DELTA, loader.getRawSize());\n+\t\t\twriteObjectHeader(Constants.OBJ_REF_DELTA, reuse.getRawSize());\n \t\t\totp.getDeltaBaseId().copyRawTo(buf, 0);\n \t\t\tout.write(buf, 0, Constants.OBJECT_ID_LENGTH);\n \t\t}\n-\t\tloader.copyRawData(out, buf, windowCursor);\n-\t\totp.disposeLoader();\n+\t\treuse.copyRawData(out, buf, windowCursor);\n \t}\n \n \tprivate void writeObjectHeader(final int objectType, long dataLength)\n@@ -955,8 +989,10 @@ boolean isWritten() {\n \t\t\treturn getOffset() != 0;\n \t\t}\n \n-\t\tPackedObjectLoader getReuseLoader() {\n-\t\t\treturn reuseLoader;\n+\t\tPackedObjectLoader useLoader() {\n+\t\t\tfinal PackedObjectLoader r = reuseLoader;\n+\t\t\treuseLoader = null;\n+\t\t\treturn r;\n \t\t}\n \n \t\tboolean hasReuseLoader() {\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackedObjectLoader.java\nindex 60333e7..6066ba1 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackedObjectLoader.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackedObjectLoader.java\n@@ -106,6 +106,30 @@ public final long getDataOffset() {\n \t}\n \n \t/**\n+\t * Peg the pack file open to support data copying.\n+\t * <p>\n+\t * Applications trying to copy raw pack data should ensure the pack stays\n+\t * open and available throughout the entire copy. To do that use:\n+\t * \n+\t * <pre>\n+\t * loader.beginCopyRawData();\n+\t * try {\n+\t * \tloader.copyRawData(out, tmpbuf, curs);\n+\t * } finally {\n+\t * \tloader.endCopyRawData();\n+\t * }\n+\t * </pre>\n+\t * \n+\t * @throws IOException\n+\t *             this loader contains stale information and cannot be used.\n+\t *             The most likely cause is the underlying pack file has been\n+\t *             deleted, and the object has moved to another pack file.\n+\t */\n+\tpublic void beginCopyRawData() throws IOException {\n+\t\tWindowCache.pin(pack);\n+\t}\n+\n+\t/**\n \t * Copy raw object representation from storage to provided output stream.\n \t * <p>\n \t * Copied data doesn't include object header. User must provide temporary\n@@ -121,12 +145,18 @@ public final long getDataOffset() {\n \t *            temporary thread storage during data access.\n \t * @throws IOException\n \t *             when the object cannot be read.\n+\t * @see #beginCopyRawData()\n \t */\n \tpublic void copyRawData(OutputStream out, byte buf[], WindowCursor curs)\n \t\t\tthrows IOException {\n \t\tpack.copyRawData(this, out, buf, curs);\n \t}\n \n+\t/** Release resources after {@link #beginCopyRawData()}. */\n+\tpublic void endCopyRawData() {\n+\t\tWindowCache.unpin(pack);\n+\t}\n+\n \t/**\n \t * @return true if this loader is capable of fast raw-data copying basing on\n \t *         compressed data checksum; false if raw-data copying needs\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java\nindex 03e531a..51d149c 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java\n@@ -223,6 +223,19 @@ public static final void get(final WindowCursor curs, final PackFile wp,\n \t\tcurs.window.ensureLoaded(curs.handle);\n \t}\n \n+\tstatic synchronized final void pin(final PackFile wp) throws IOException {\n+\t\tif (++wp.openCount == 1) {\n+\t\t\topenFile(wp);\n+\t\t}\n+\t}\n+\n+\tstatic synchronized final void unpin(final PackFile wp) {\n+\t\tif (--wp.openCount == 0) {\n+\t\t\topenFileCount--;\n+\t\t\twp.cacheClose();\n+\t\t}\n+\t}\n+\n \tprivate static synchronized final void getImpl(final WindowCursor curs,\n \t\t\tfinal PackFile wp, final long position) throws IOException {\n \t\tfinal int id = (int) (position >> windowSizeShift);\n@@ -241,27 +254,7 @@ private static synchronized final void getImpl(final WindowCursor curs,\n \t\t}\n \n \t\tif (wp.openCount == 0) {\n-\t\t\ttry {\n-\t\t\t\topenFileCount++;\n-\t\t\t\treleaseMemory();\n-\t\t\t\trunClearedWindowQueue();\n-\t\t\t\twp.openCount = 1;\n-\t\t\t\twp.cacheOpen();\n-\t\t\t} catch (IOException ioe) {\n-\t\t\t\topenFileCount--;\n-\t\t\t\twp.openCount = 0;\n-\t\t\t\tthrow ioe;\n-\t\t\t} catch (RuntimeException ioe) {\n-\t\t\t\topenFileCount--;\n-\t\t\t\twp.openCount = 0;\n-\t\t\t\tthrow ioe;\n-\t\t\t} catch (Error ioe) {\n-\t\t\t\topenFileCount--;\n-\t\t\t\twp.openCount = 0;\n-\t\t\t\tthrow ioe;\n-\t\t\t} finally {\n-\t\t\t\twp.openCount--;\n-\t\t\t}\n+\t\t\topenFile(wp);\n \n \t\t\t// The cacheOpen may have mapped the window we are trying to\n \t\t\t// map ourselves. Retrying the search ensures that does not\n@@ -294,6 +287,30 @@ private static synchronized final void getImpl(final WindowCursor curs,\n \t\tinsertLRU(e);\n \t}\n \n+\tprivate static void openFile(final PackFile wp) throws IOException {\n+\t\ttry {\n+\t\t\topenFileCount++;\n+\t\t\treleaseMemory();\n+\t\t\trunClearedWindowQueue();\n+\t\t\twp.openCount = 1;\n+\t\t\twp.cacheOpen();\n+\t\t} catch (IOException ioe) {\n+\t\t\topenFileCount--;\n+\t\t\twp.openCount = 0;\n+\t\t\tthrow ioe;\n+\t\t} catch (RuntimeException ioe) {\n+\t\t\topenFileCount--;\n+\t\t\twp.openCount = 0;\n+\t\t\tthrow ioe;\n+\t\t} catch (Error ioe) {\n+\t\t\topenFileCount--;\n+\t\t\twp.openCount = 0;\n+\t\t\tthrow ioe;\n+\t\t} finally {\n+\t\t\twp.openCount--;\n+\t\t}\n+\t}\n+\n \tstatic synchronized void markLoaded(final ByteWindow w) {\n \t\tif (--w.provider.openCount == 0) {\n \t\t\topenFileCount--;\n-- \n1.6.3.rc1.205.g37f8\n"}]}