Ensure ObjectReader used by PackWriter is released
The ObjectReader API demands that we release the reader when we are
done with it. PackWriter contains a reader, which it uses for the
entire packing session. Expose the release of the reader through
a release method on the writer.
This still doesn't address the RevWalk and TreeWalk users, who
don't correctly release their reader. But its a small step in the
right direction.
Change-Id: I5cb0b5c1b432434a799fceb21b86479e09b84a0a
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java
index 18275ec..5b0e74c 100644
--- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java
+++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java
@@ -600,33 +600,38 @@ private static void assertHash(RevObject id, byte[] bin) {
public void packAndPrune() throws Exception {
if (db.getObjectDatabase() instanceof ObjectDirectory) {
ObjectDirectory odb = (ObjectDirectory) db.getObjectDatabase();
+
+ final File pack, idx;
PackWriter pw = new PackWriter(db, NullProgressMonitor.INSTANCE);
-
- Set<ObjectId> all = new HashSet<ObjectId>();
- for (Ref r : db.getAllRefs().values())
- all.add(r.getObjectId());
- pw.preparePack(all, Collections.<ObjectId> emptySet());
-
- final ObjectId name = pw.computeName();
- OutputStream out;
-
- final File pack = nameFor(odb, name, ".pack");
- out = new BufferedOutputStream(new FileOutputStream(pack));
try {
- pw.writePack(out);
- } finally {
- out.close();
- }
- pack.setReadOnly();
+ Set<ObjectId> all = new HashSet<ObjectId>();
+ for (Ref r : db.getAllRefs().values())
+ all.add(r.getObjectId());
+ pw.preparePack(all, Collections.<ObjectId> emptySet());
- final File idx = nameFor(odb, name, ".idx");
- out = new BufferedOutputStream(new FileOutputStream(idx));
- try {
- pw.writeIndex(out);
+ final ObjectId name = pw.computeName();
+ OutputStream out;
+
+ pack = nameFor(odb, name, ".pack");
+ out = new BufferedOutputStream(new FileOutputStream(pack));
+ try {
+ pw.writePack(out);
+ } finally {
+ out.close();
+ }
+ pack.setReadOnly();
+
+ idx = nameFor(odb, name, ".idx");
+ out = new BufferedOutputStream(new FileOutputStream(idx));
+ try {
+ pw.writeIndex(out);
+ } finally {
+ out.close();
+ }
+ idx.setReadOnly();
} finally {
- out.close();
+ pw.release();
}
- idx.setReadOnly();
odb.openPack(pack, idx);
updateServerInfo();
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java
index 96b1178..d85903c 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java
@@ -138,6 +138,7 @@ public void testObjectMovedWithinPack()
pw.addObject(o2);
pw.addObject(o1);
write(out1, pw);
+ pw.release();
// Try the old name, then the new name. The old name should cause the
// pack to reload when it opens and the index and pack mismatch.
@@ -208,6 +209,7 @@ private RevObject parse(final AnyObjectId id)
final File idxFile = fullPackFileName(name, ".idx");
final File[] files = new File[] { packFile, idxFile };
write(files, pw);
+ pw.release();
return files;
}
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/PackWriterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/PackWriterTest.java
index b12359c..19eec5f 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/PackWriterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/PackWriterTest.java
@@ -484,6 +484,7 @@ private void createVerifyOpenPack(final Collection<ObjectId> interestings,
writer.setIgnoreMissingUninteresting(ignoreMissingUninteresting);
writer.preparePack(interestings, uninterestings);
writer.writePack(os);
+ writer.release();
verifyOpenPack(thin);
}
@@ -491,6 +492,7 @@ private void createVerifyOpenPack(final Iterator<RevObject> objectSource)
throws MissingObjectException, IOException {
writer.preparePack(objectSource);
writer.writePack(os);
+ writer.release();
verifyOpenPack(false);
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java
index a716845..cf6e382 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/PackWriter.java
@@ -610,22 +610,24 @@ private List<ObjectToPack> sortByName() {
* stream.
*/
public void writePack(OutputStream packStream) throws IOException {
- try {
- if ((reuseDeltas || reuseObjects) && reuseSupport != null)
- searchForReuse();
+ if ((reuseDeltas || reuseObjects) && reuseSupport != null)
+ searchForReuse();
- out = new PackOutputStream(packStream, isDeltaBaseAsOffset());
+ out = new PackOutputStream(packStream, isDeltaBaseAsOffset());
- int cnt = getObjectsNumber();
- writeMonitor.beginTask(WRITING_OBJECTS_PROGRESS, cnt);
- out.writeFileHeader(PACK_VERSION_GENERATED, cnt);
- writeObjects();
- writeChecksum();
- writeMonitor.endTask();
- } finally {
- out = null;
- reader.release();
- }
+ writeMonitor.beginTask(WRITING_OBJECTS_PROGRESS, getObjectsNumber());
+ out.writeFileHeader(PACK_VERSION_GENERATED, getObjectsNumber());
+ writeObjects();
+ writeChecksum();
+
+ out = null;
+ reader.release();
+ writeMonitor.endTask();
+ }
+
+ /** Release all resources used by this writer. */
+ public void release() {
+ reader.release();
}
private void searchForReuse() throws IOException {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java
index 72460c9..d1f7bfc 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java
@@ -48,6 +48,7 @@
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.List;
import java.util.Map;
import org.eclipse.jgit.JGitText;
@@ -226,25 +227,29 @@ private String enableCapabilities(final ProgressMonitor monitor) {
private void writePack(final Map<String, RemoteRefUpdate> refUpdates,
final ProgressMonitor monitor) throws IOException {
+ List<ObjectId> remoteObjects = new ArrayList<ObjectId>(getRefs().size());
+ List<ObjectId> newObjects = new ArrayList<ObjectId>(refUpdates.size());
+
+ final long start;
final PackWriter writer = new PackWriter(local, monitor);
- final ArrayList<ObjectId> remoteObjects = new ArrayList<ObjectId>(
- getRefs().size());
- final ArrayList<ObjectId> newObjects = new ArrayList<ObjectId>(
- refUpdates.size());
+ try {
- for (final Ref r : getRefs())
- remoteObjects.add(r.getObjectId());
- remoteObjects.addAll(additionalHaves);
- for (final RemoteRefUpdate r : refUpdates.values()) {
- if (!ObjectId.zeroId().equals(r.getNewObjectId()))
- newObjects.add(r.getNewObjectId());
+ for (final Ref r : getRefs())
+ remoteObjects.add(r.getObjectId());
+ remoteObjects.addAll(additionalHaves);
+ for (final RemoteRefUpdate r : refUpdates.values()) {
+ if (!ObjectId.zeroId().equals(r.getNewObjectId()))
+ newObjects.add(r.getNewObjectId());
+ }
+
+ writer.setThin(thinPack);
+ writer.setDeltaBaseAsOffset(capableOfsDelta);
+ writer.preparePack(newObjects, remoteObjects);
+ start = System.currentTimeMillis();
+ writer.writePack(out);
+ } finally {
+ writer.release();
}
-
- writer.setThin(thinPack);
- writer.setDeltaBaseAsOffset(capableOfsDelta);
- writer.preparePack(newObjects, remoteObjects);
- final long start = System.currentTimeMillis();
- writer.writePack(out);
out.flush();
packTransferTime = System.currentTimeMillis() - start;
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BundleWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BundleWriter.java
index c5e9baa..71d58e1 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BundleWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BundleWriter.java
@@ -165,37 +165,41 @@ public void assume(final RevCommit c) {
* stream.
*/
public void writeBundle(OutputStream os) throws IOException {
- final HashSet<ObjectId> inc = new HashSet<ObjectId>();
- final HashSet<ObjectId> exc = new HashSet<ObjectId>();
- inc.addAll(include.values());
- for (final RevCommit r : assume)
- exc.add(r.getId());
- packWriter.setThin(exc.size() > 0);
- packWriter.preparePack(inc, exc);
+ try {
+ final HashSet<ObjectId> inc = new HashSet<ObjectId>();
+ final HashSet<ObjectId> exc = new HashSet<ObjectId>();
+ inc.addAll(include.values());
+ for (final RevCommit r : assume)
+ exc.add(r.getId());
+ packWriter.setThin(exc.size() > 0);
+ packWriter.preparePack(inc, exc);
- final Writer w = new OutputStreamWriter(os, Constants.CHARSET);
- w.write(TransportBundle.V2_BUNDLE_SIGNATURE);
- w.write('\n');
+ final Writer w = new OutputStreamWriter(os, Constants.CHARSET);
+ w.write(TransportBundle.V2_BUNDLE_SIGNATURE);
+ w.write('\n');
- final char[] tmp = new char[Constants.OBJECT_ID_STRING_LENGTH];
- for (final RevCommit a : assume) {
- w.write('-');
- a.copyTo(tmp, w);
- if (a.getRawBuffer() != null) {
- w.write(' ');
- w.write(a.getShortMessage());
+ final char[] tmp = new char[Constants.OBJECT_ID_STRING_LENGTH];
+ for (final RevCommit a : assume) {
+ w.write('-');
+ a.copyTo(tmp, w);
+ if (a.getRawBuffer() != null) {
+ w.write(' ');
+ w.write(a.getShortMessage());
+ }
+ w.write('\n');
}
- w.write('\n');
- }
- for (final Map.Entry<String, ObjectId> e : include.entrySet()) {
- e.getValue().copyTo(tmp, w);
- w.write(' ');
- w.write(e.getKey());
- w.write('\n');
- }
+ for (final Map.Entry<String, ObjectId> e : include.entrySet()) {
+ e.getValue().copyTo(tmp, w);
+ w.write(' ');
+ w.write(e.getKey());
+ w.write('\n');
+ }
- w.write('\n');
- w.flush();
- packWriter.writePack(os);
+ w.write('\n');
+ w.flush();
+ packWriter.writePack(os);
+ } finally {
+ packWriter.release();
+ }
}
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index 17c37cf..dfb4168 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -569,25 +569,29 @@ private void sendPack() throws IOException {
final PackWriter pw;
pw = new PackWriter(db, pm, NullProgressMonitor.INSTANCE);
- pw.setDeltaBaseAsOffset(options.contains(OPTION_OFS_DELTA));
- pw.setThin(thin);
- pw.preparePack(wantAll, commonBase);
- if (options.contains(OPTION_INCLUDE_TAG)) {
- for (final Ref r : refs.values()) {
- final RevObject o;
- try {
- o = walk.parseAny(r.getObjectId());
- } catch (IOException e) {
- continue;
+ try {
+ pw.setDeltaBaseAsOffset(options.contains(OPTION_OFS_DELTA));
+ pw.setThin(thin);
+ pw.preparePack(wantAll, commonBase);
+ if (options.contains(OPTION_INCLUDE_TAG)) {
+ for (final Ref r : refs.values()) {
+ final RevObject o;
+ try {
+ o = walk.parseAny(r.getObjectId());
+ } catch (IOException e) {
+ continue;
+ }
+ if (o.has(WANT) || !(o instanceof RevTag))
+ continue;
+ final RevTag t = (RevTag) o;
+ if (!pw.willInclude(t) && pw.willInclude(t.getObject()))
+ pw.addObject(t);
}
- if (o.has(WANT) || !(o instanceof RevTag))
- continue;
- final RevTag t = (RevTag) o;
- if (!pw.willInclude(t) && pw.willInclude(t.getObject()))
- pw.addObject(t);
}
+ pw.writePack(packOut);
+ } finally {
+ pw.release();
}
- pw.writePack(packOut);
packOut.flush();
if (sideband)
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkPushConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkPushConnection.java
index 1ab6081..a9f9d60 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkPushConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkPushConnection.java
@@ -209,8 +209,8 @@ private void sendpack(final List<RemoteRefUpdate> updates,
String pathPack = null;
String pathIdx = null;
+ final PackWriter pw = new PackWriter(local, monitor);
try {
- final PackWriter pw = new PackWriter(local, monitor);
final List<ObjectId> need = new ArrayList<ObjectId>();
final List<ObjectId> have = new ArrayList<ObjectId>();
for (final RemoteRefUpdate r : updates)
@@ -281,6 +281,8 @@ private void sendpack(final List<RemoteRefUpdate> updates,
safeDelete(pathPack);
throw new TransportException(uri, JGitText.get().cannotStoreObjects, err);
+ } finally {
+ pw.release();
}
}
