From 515deaf7e503738b4c53c3c2dfd6d7acab3bef18 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Tue, 29 Jun 2010 15:12:51 -0700 Subject: [PATCH] Ensure RevWalk is released when done Update a number of calling sites of RevWalk to ensure the walker's internal ObjectReader is released after the walk is no longer used. Because the ObjectReader is likely to hold onto a native resource like an Inflater, we don't want to leak them outside of their useful scope. Where possible we also try to share ObjectReaders across several walk pools, or between a walker and a PackWriter. This permits the ObjectReader to actually do some caching if it felt inclined to do so. Not everything was updated, we'll probably need to come back and update even more call sites, but these are some of the biggest offenders. Test cases in particular aren't updated. My plan is to move most storage-agnostic tests onto some purely in-memory storage solution that doesn't do compression. Change-Id: I04087ec79faeea208b19848939898ad7172b6672 Signed-off-by: Shawn O. Pearce --- .../jgit/http/server/InfoRefsServlet.java | 49 +++++----- .../jgit/http/server/ReceivePackServlet.java | 7 +- .../jgit/http/server/UploadPackServlet.java | 7 +- .../jgit/pgm/debug/RebuildCommitGraph.java | 1 + .../org/eclipse/jgit/api/CommitCommand.java | 64 ++++++------- .../org/eclipse/jgit/api/MergeCommand.java | 66 +++++++------- .../src/org/eclipse/jgit/lib/RefUpdate.java | 14 ++- .../src/org/eclipse/jgit/lib/Repository.java | 10 ++- .../jgit/storage/file/RefDirectory.java | 29 +++--- .../jgit/storage/file/RefDirectoryRename.java | 3 +- .../eclipse/jgit/storage/pack/PackWriter.java | 43 +++++++-- .../transport/BasePackFetchConnection.java | 6 ++ .../jgit/transport/BundleFetchConnection.java | 90 ++++++++++--------- .../eclipse/jgit/transport/FetchProcess.java | 37 +++++--- .../eclipse/jgit/transport/PushProcess.java | 52 ++++++----- .../eclipse/jgit/transport/ReceivePack.java | 1 + .../eclipse/jgit/transport/UploadPack.java | 3 +- 17 files changed, 298 insertions(+), 184 deletions(-) diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/InfoRefsServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/InfoRefsServlet.java index f667ce95a7..647919e065 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/InfoRefsServlet.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/InfoRefsServlet.java @@ -74,30 +74,35 @@ class InfoRefsServlet extends HttpServlet { final Repository db = getRepository(req); final RevWalk walk = new RevWalk(db); - final RevFlag ADVERTISED = walk.newFlag("ADVERTISED"); + try { + final RevFlag ADVERTISED = walk.newFlag("ADVERTISED"); - final OutputStreamWriter out = new OutputStreamWriter( - new SmartOutputStream(req, rsp), Constants.CHARSET); - final RefAdvertiser adv = new RefAdvertiser() { - @Override - protected void writeOne(final CharSequence line) throws IOException { - // Whoever decided that info/refs should use a different - // delimiter than the native git:// protocol shouldn't - // be allowed to design this sort of stuff. :-( - out.append(line.toString().replace(' ', '\t')); - } + final OutputStreamWriter out = new OutputStreamWriter( + new SmartOutputStream(req, rsp), Constants.CHARSET); + final RefAdvertiser adv = new RefAdvertiser() { + @Override + protected void writeOne(final CharSequence line) + throws IOException { + // Whoever decided that info/refs should use a different + // delimiter than the native git:// protocol shouldn't + // be allowed to design this sort of stuff. :-( + out.append(line.toString().replace(' ', '\t')); + } - @Override - protected void end() { - // No end marker required for info/refs format. - } - }; - adv.init(walk, ADVERTISED); - adv.setDerefTags(true); + @Override + protected void end() { + // No end marker required for info/refs format. + } + }; + adv.init(walk, ADVERTISED); + adv.setDerefTags(true); - Map refs = db.getAllRefs(); - refs.remove(Constants.HEAD); - adv.send(refs); - out.close(); + Map refs = db.getAllRefs(); + refs.remove(Constants.HEAD); + adv.send(refs); + out.close(); + } finally { + walk.release(); + } } } diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java index 49fd535a71..4bc05c1886 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java @@ -83,7 +83,12 @@ class ReceivePackServlet extends HttpServlet { protected void advertise(HttpServletRequest req, Repository db, PacketLineOutRefAdvertiser pck) throws IOException, ServiceNotEnabledException, ServiceNotAuthorizedException { - receivePackFactory.create(req, db).sendAdvertisedRefs(pck); + ReceivePack rp = receivePackFactory.create(req, db); + try { + rp.sendAdvertisedRefs(pck); + } finally { + rp.getRevWalk().release(); + } } } diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java index 6d0d64fc63..602d66a90c 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java @@ -83,7 +83,12 @@ class UploadPackServlet extends HttpServlet { protected void advertise(HttpServletRequest req, Repository db, PacketLineOutRefAdvertiser pck) throws IOException, ServiceNotEnabledException, ServiceNotAuthorizedException { - uploadPackFactory.create(req, db).sendAdvertisedRefs(pck); + UploadPack up = uploadPackFactory.create(req, db); + try { + up.sendAdvertisedRefs(pck); + } finally { + up.getRevWalk().release(); + } } } diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/RebuildCommitGraph.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/RebuildCommitGraph.java index 8ba3e4b90a..5b75c1b5c5 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/RebuildCommitGraph.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/debug/RebuildCommitGraph.java @@ -297,6 +297,7 @@ class RebuildCommitGraph extends TextBuiltin { name, id)); } } finally { + rw.release(); br.close(); } return refs; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java index 1cf2fe669c..c2db140b05 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java @@ -160,38 +160,42 @@ public class CommitCommand extends GitCommand { .format(commit)); odi.flush(); - RevCommit revCommit = new RevWalk(repo) - .parseCommit(commitId); - RefUpdate ru = repo.updateRef(Constants.HEAD); - ru.setNewObjectId(commitId); - ru.setRefLogMessage("commit : " - + revCommit.getShortMessage(), false); + RevWalk revWalk = new RevWalk(repo); + try { + RevCommit revCommit = revWalk.parseCommit(commitId); + RefUpdate ru = repo.updateRef(Constants.HEAD); + ru.setNewObjectId(commitId); + ru.setRefLogMessage("commit : " + + revCommit.getShortMessage(), false); - ru.setExpectedOldObjectId(headId); - Result rc = ru.update(); - switch (rc) { - case NEW: - case FAST_FORWARD: { - setCallable(false); - File meta = repo.getDirectory(); - if (state == RepositoryState.MERGING_RESOLVED - && meta != null) { - // Commit was successful. Now delete the files - // used for merge commits - new File(meta, Constants.MERGE_HEAD).delete(); - new File(meta, Constants.MERGE_MSG).delete(); + ru.setExpectedOldObjectId(headId); + Result rc = ru.update(); + switch (rc) { + case NEW: + case FAST_FORWARD: { + setCallable(false); + File meta = repo.getDirectory(); + if (state == RepositoryState.MERGING_RESOLVED + && meta != null) { + // Commit was successful. Now delete the files + // used for merge commits + new File(meta, Constants.MERGE_HEAD).delete(); + new File(meta, Constants.MERGE_MSG).delete(); + } + return revCommit; } - return revCommit; - } - case REJECTED: - case LOCK_FAILURE: - throw new ConcurrentRefUpdateException( - JGitText.get().couldNotLockHEAD, ru.getRef(), - rc); - default: - throw new JGitInternalException(MessageFormat.format( - JGitText.get().updatingRefFailed, - Constants.HEAD, commitId.toString(), rc)); + case REJECTED: + case LOCK_FAILURE: + throw new ConcurrentRefUpdateException(JGitText + .get().couldNotLockHEAD, ru.getRef(), rc); + default: + throw new JGitInternalException(MessageFormat + .format(JGitText.get().updatingRefFailed, + Constants.HEAD, + commitId.toString(), rc)); + } + } finally { + revWalk.release(); } } finally { odi.release(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java index 76a3bc4794..972aa618ad 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeCommand.java @@ -119,37 +119,41 @@ public class MergeCommand extends GitCommand { // Check for FAST_FORWARD, ALREADY_UP_TO_DATE RevWalk revWalk = new RevWalk(repo); - RevCommit headCommit = revWalk.lookupCommit(head.getObjectId()); - - Ref ref = commits.get(0); - - refLogMessage.append(ref.getName()); - - // handle annotated tags - ObjectId objectId = ref.getPeeledObjectId(); - if (objectId == null) - objectId = ref.getObjectId(); - - RevCommit srcCommit = revWalk.lookupCommit(objectId); - if (revWalk.isMergedInto(srcCommit, headCommit)) { - setCallable(false); - return new MergeResult(headCommit, - MergeStatus.ALREADY_UP_TO_DATE, mergeStrategy); - } else if (revWalk.isMergedInto(headCommit, srcCommit)) { - // FAST_FORWARD detected: skip doing a real merge but only - // update HEAD - refLogMessage.append(": " + MergeStatus.FAST_FORWARD); - checkoutNewHead(revWalk, headCommit, srcCommit); - updateHead(refLogMessage, srcCommit, head.getObjectId()); - setCallable(false); - return new MergeResult(srcCommit, MergeStatus.FAST_FORWARD, - mergeStrategy); - } else { - return new MergeResult( - headCommit, - MergeResult.MergeStatus.NOT_SUPPORTED, - mergeStrategy, - JGitText.get().onlyAlreadyUpToDateAndFastForwardMergesAreAvailable); + try { + RevCommit headCommit = revWalk.lookupCommit(head.getObjectId()); + + Ref ref = commits.get(0); + + refLogMessage.append(ref.getName()); + + // handle annotated tags + ObjectId objectId = ref.getPeeledObjectId(); + if (objectId == null) + objectId = ref.getObjectId(); + + RevCommit srcCommit = revWalk.lookupCommit(objectId); + if (revWalk.isMergedInto(srcCommit, headCommit)) { + setCallable(false); + return new MergeResult(headCommit, + MergeStatus.ALREADY_UP_TO_DATE, mergeStrategy); + } else if (revWalk.isMergedInto(headCommit, srcCommit)) { + // FAST_FORWARD detected: skip doing a real merge but only + // update HEAD + refLogMessage.append(": " + MergeStatus.FAST_FORWARD); + checkoutNewHead(revWalk, headCommit, srcCommit); + updateHead(refLogMessage, srcCommit, head.getObjectId()); + setCallable(false); + return new MergeResult(srcCommit, MergeStatus.FAST_FORWARD, + mergeStrategy); + } else { + return new MergeResult( + headCommit, + MergeResult.MergeStatus.NOT_SUPPORTED, + mergeStrategy, + JGitText.get().onlyAlreadyUpToDateAndFastForwardMergesAreAvailable); + } + } finally { + revWalk.release(); } } catch (IOException e) { throw new JGitInternalException( diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java index e04a587ac2..e6f8933389 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java @@ -440,7 +440,12 @@ public abstract class RefUpdate { * an unexpected IO error occurred while writing changes. */ public Result update() throws IOException { - return update(new RevWalk(getRepository())); + RevWalk rw = new RevWalk(getRepository()); + try { + return update(rw); + } finally { + rw.release(); + } } /** @@ -485,7 +490,12 @@ public abstract class RefUpdate { * @throws IOException */ public Result delete() throws IOException { - return delete(new RevWalk(getRepository())); + RevWalk rw = new RevWalk(getRepository()); + try { + return delete(rw); + } finally { + rw.release(); + } } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index aeb160e521..cb663aee32 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java @@ -484,9 +484,17 @@ public abstract class Repository { * on serious errors */ public ObjectId resolve(final String revstr) throws IOException { + RevWalk rw = new RevWalk(this); + try { + return resolve(rw, revstr); + } finally { + rw.release(); + } + } + + private ObjectId resolve(final RevWalk rw, final String revstr) throws IOException { char[] rev = revstr.toCharArray(); RevObject ref = null; - RevWalk rw = new RevWalk(this); for (int i = 0; i < rev.length; ++i) { switch (rev[i]) { case '^': diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java index f7ffa3e39a..68b0270df9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java @@ -74,6 +74,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jgit.JGitText; +import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.ObjectWritingException; import org.eclipse.jgit.events.RefsChangedEvent; import org.eclipse.jgit.lib.Constants; @@ -418,16 +419,7 @@ public class RefDirectory extends RefDatabase { if (leaf.isPeeled() || leaf.getObjectId() == null) return ref; - RevWalk rw = new RevWalk(getRepository()); - RevObject obj = rw.parseAny(leaf.getObjectId()); - ObjectIdRef newLeaf; - if (obj instanceof RevTag) { - newLeaf = new ObjectIdRef.PeeledTag(leaf.getStorage(), leaf - .getName(), leaf.getObjectId(), rw.peel(obj).copy()); - } else { - newLeaf = new ObjectIdRef.PeeledNonTag(leaf.getStorage(), leaf - .getName(), leaf.getObjectId()); - } + ObjectIdRef newLeaf = doPeel(leaf); // Try to remember this peeling in the cache, so we don't have to do // it again in the future, but only if the reference is unchanged. @@ -444,6 +436,23 @@ public class RefDirectory extends RefDatabase { return recreate(ref, newLeaf); } + private ObjectIdRef doPeel(final Ref leaf) throws MissingObjectException, + IOException { + RevWalk rw = new RevWalk(getRepository()); + try { + RevObject obj = rw.parseAny(leaf.getObjectId()); + if (obj instanceof RevTag) { + return new ObjectIdRef.PeeledTag(leaf.getStorage(), leaf + .getName(), leaf.getObjectId(), rw.peel(obj).copy()); + } else { + return new ObjectIdRef.PeeledNonTag(leaf.getStorage(), leaf + .getName(), leaf.getObjectId()); + } + } finally { + rw.release(); + } + } + private static Ref recreate(final Ref old, final ObjectIdRef leaf) { if (old.isSymbolic()) { Ref dst = recreate(old.getTarget(), leaf); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectoryRename.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectoryRename.java index b43b70f1e4..4f3efe343e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectoryRename.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectoryRename.java @@ -92,10 +92,10 @@ class RefDirectoryRename extends RefRename { if (source.getRef().isSymbolic()) return Result.IO_FAILURE; // not supported - final RevWalk rw = new RevWalk(refdb.getRepository()); objId = source.getOldObjectId(); updateHEAD = needToUpdateHEAD(); tmp = refdb.newTemporaryUpdate(); + final RevWalk rw = new RevWalk(refdb.getRepository()); try { // First backup the source so its never unreachable. tmp.setNewObjectId(objId); @@ -177,6 +177,7 @@ class RefDirectoryRename extends RefRename { } catch (IOException err) { refdb.fileFor(tmp.getName()).delete(); } + rw.release(); } } 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 3fab3f7470..c851238c9c 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 @@ -63,6 +63,7 @@ import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.StoredObjectRepresentationNotAvailableException; import org.eclipse.jgit.lib.AnyObjectId; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.CoreConfig; import org.eclipse.jgit.lib.NullProgressMonitor; @@ -181,8 +182,6 @@ public class PackWriter { // edge objects for thin packs private final ObjectIdSubclassMap edgeObjects = new ObjectIdSubclassMap(); - private final Repository db; - private final Deflater deflater; private final ObjectReader reader; @@ -218,19 +217,51 @@ public class PackWriter { * repository where objects are stored. */ public PackWriter(final Repository repo) { - this.db = repo; + this(repo, repo.newObjectReader()); + } - reader = db.newObjectReader(); + /** + * Create a writer to load objects from the specified reader. + *

+ * Objects for packing are specified in {@link #preparePack(Iterator)} or + * {@link #preparePack(ProgressMonitor, Collection, Collection)}. + * + * @param reader + * reader to read from the repository with. + */ + public PackWriter(final ObjectReader reader) { + this(null, reader); + } + + /** + * Create writer for specified repository. + *

+ * Objects for packing are specified in {@link #preparePack(Iterator)} or + * {@link #preparePack(ProgressMonitor, Collection, Collection)}. + * + * @param repo + * repository where objects are stored. + * @param reader + * reader to read from the repository with. + */ + public PackWriter(final Repository repo, final ObjectReader reader) { + this.reader = reader; if (reader instanceof ObjectReuseAsIs) reuseSupport = ((ObjectReuseAsIs) reader); else reuseSupport = null; - final CoreConfig coreConfig = db.getConfig().get(CoreConfig.KEY); - this.deflater = new Deflater(coreConfig.getCompression()); + final CoreConfig coreConfig = configOf(repo).get(CoreConfig.KEY); + deflater = new Deflater(coreConfig.getCompression()); outputVersion = coreConfig.getPackIndexVersion(); } + private static Config configOf(final Repository repo) { + if (repo == null) + return new Config(); + return repo.getConfig(); + } + /** * Check whether object is configured to reuse deltas existing in * repository. diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java index 8c336c5255..af18f18d8e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java @@ -270,6 +270,12 @@ abstract class BasePackFetchConnection extends BasePackConnection implements } } + @Override + public void close() { + walk.release(); + super.close(); + } + private int maxTimeWanted(final Collection wants) { int maxTime = 0; for (final Ref r : wants) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BundleFetchConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BundleFetchConnection.java index 98ecc5540f..126acab48d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BundleFetchConnection.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BundleFetchConnection.java @@ -213,57 +213,65 @@ class BundleFetchConnection extends BaseFetchConnection { return; final RevWalk rw = new RevWalk(transport.local); - final RevFlag PREREQ = rw.newFlag("PREREQ"); - final RevFlag SEEN = rw.newFlag("SEEN"); + try { + final RevFlag PREREQ = rw.newFlag("PREREQ"); + final RevFlag SEEN = rw.newFlag("SEEN"); - final Map missing = new HashMap(); - final List commits = new ArrayList(); - for (final Map.Entry e : prereqs.entrySet()) { - ObjectId p = e.getKey(); - try { - final RevCommit c = rw.parseCommit(p); - if (!c.has(PREREQ)) { - c.add(PREREQ); - commits.add(c); + final Map missing = new HashMap(); + final List commits = new ArrayList(); + for (final Map.Entry e : prereqs.entrySet()) { + ObjectId p = e.getKey(); + try { + final RevCommit c = rw.parseCommit(p); + if (!c.has(PREREQ)) { + c.add(PREREQ); + commits.add(c); + } + } catch (MissingObjectException notFound) { + missing.put(p, e.getValue()); + } catch (IOException err) { + throw new TransportException(transport.uri, MessageFormat + .format(JGitText.get().cannotReadCommit, p.name()), + err); } - } catch (MissingObjectException notFound) { - missing.put(p, e.getValue()); - } catch (IOException err) { - throw new TransportException(transport.uri - , MessageFormat.format(JGitText.get().cannotReadCommit, p.name()), err); } - } - if (!missing.isEmpty()) - throw new MissingBundlePrerequisiteException(transport.uri, missing); + if (!missing.isEmpty()) + throw new MissingBundlePrerequisiteException(transport.uri, + missing); - for (final Ref r : transport.local.getAllRefs().values()) { - try { - rw.markStart(rw.parseCommit(r.getObjectId())); - } catch (IOException readError) { - // If we cannot read the value of the ref skip it. + for (final Ref r : transport.local.getAllRefs().values()) { + try { + rw.markStart(rw.parseCommit(r.getObjectId())); + } catch (IOException readError) { + // If we cannot read the value of the ref skip it. + } } - } - int remaining = commits.size(); - try { - RevCommit c; - while ((c = rw.next()) != null) { - if (c.has(PREREQ)) { - c.add(SEEN); - if (--remaining == 0) - break; + int remaining = commits.size(); + try { + RevCommit c; + while ((c = rw.next()) != null) { + if (c.has(PREREQ)) { + c.add(SEEN); + if (--remaining == 0) + break; + } } + } catch (IOException err) { + throw new TransportException(transport.uri, + JGitText.get().cannotReadObject, err); } - } catch (IOException err) { - throw new TransportException(transport.uri, JGitText.get().cannotReadObject, err); - } - if (remaining > 0) { - for (final RevObject o : commits) { - if (!o.has(SEEN)) - missing.put(o, prereqs.get(o)); + if (remaining > 0) { + for (final RevObject o : commits) { + if (!o.has(SEEN)) + missing.put(o, prereqs.get(o)); + } + throw new MissingBundlePrerequisiteException(transport.uri, + missing); } - throw new MissingBundlePrerequisiteException(transport.uri, missing); + } finally { + rw.release(); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java index 72d73eb59b..ca68858059 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java @@ -176,16 +176,21 @@ class FetchProcess { } final RevWalk walk = new RevWalk(transport.local); - if (transport.isRemoveDeletedRefs()) - deleteStaleTrackingRefs(result, walk); - for (TrackingRefUpdate u : localUpdates) { - try { - u.update(walk); - result.add(u); - } catch (IOException err) { - throw new TransportException(MessageFormat.format( - JGitText.get().failureUpdatingTrackingRef, u.getLocalName(), err.getMessage()), err); + try { + if (transport.isRemoveDeletedRefs()) + deleteStaleTrackingRefs(result, walk); + for (TrackingRefUpdate u : localUpdates) { + try { + u.update(walk); + result.add(u); + } catch (IOException err) { + throw new TransportException(MessageFormat.format(JGitText + .get().failureUpdatingTrackingRef, + u.getLocalName(), err.getMessage()), err); + } } + } finally { + walk.release(); } if (!fetchHeadUpdates.isEmpty()) { @@ -296,11 +301,15 @@ class FetchProcess { private boolean askForIsComplete() throws TransportException { try { final ObjectWalk ow = new ObjectWalk(transport.local); - for (final ObjectId want : askFor.keySet()) - ow.markStart(ow.parseAny(want)); - for (final Ref ref : transport.local.getAllRefs().values()) - ow.markUninteresting(ow.parseAny(ref.getObjectId())); - ow.checkConnectivity(); + try { + for (final ObjectId want : askFor.keySet()) + ow.markStart(ow.parseAny(want)); + for (final Ref ref : transport.local.getAllRefs().values()) + ow.markUninteresting(ow.parseAny(ref.getObjectId())); + ow.checkConnectivity(); + } finally { + ow.release(); + } return true; } catch (MissingObjectException e) { return false; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java index 02497cb06f..6cd796a0dc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushProcess.java @@ -122,32 +122,38 @@ class PushProcess { */ PushResult execute(final ProgressMonitor monitor) throws NotSupportedException, TransportException { - monitor.beginTask(PROGRESS_OPENING_CONNECTION, ProgressMonitor.UNKNOWN); - - final PushResult res = new PushResult(); - connection = transport.openPush(); try { - res.setAdvertisedRefs(transport.getURI(), connection.getRefsMap()); - res.setRemoteUpdates(toPush); - monitor.endTask(); - - final Map preprocessed = prepareRemoteUpdates(); - if (transport.isDryRun()) - modifyUpdatesForDryRun(); - else if (!preprocessed.isEmpty()) - connection.push(monitor, preprocessed); + monitor.beginTask(PROGRESS_OPENING_CONNECTION, + ProgressMonitor.UNKNOWN); + + final PushResult res = new PushResult(); + connection = transport.openPush(); + try { + res.setAdvertisedRefs(transport.getURI(), connection + .getRefsMap()); + res.setRemoteUpdates(toPush); + monitor.endTask(); + + final Map preprocessed = prepareRemoteUpdates(); + if (transport.isDryRun()) + modifyUpdatesForDryRun(); + else if (!preprocessed.isEmpty()) + connection.push(monitor, preprocessed); + } finally { + connection.close(); + res.addMessages(connection.getMessages()); + } + if (!transport.isDryRun()) + updateTrackingRefs(); + for (final RemoteRefUpdate rru : toPush.values()) { + final TrackingRefUpdate tru = rru.getTrackingRefUpdate(); + if (tru != null) + res.add(tru); + } + return res; } finally { - connection.close(); - res.addMessages(connection.getMessages()); - } - if (!transport.isDryRun()) - updateTrackingRefs(); - for (final RemoteRefUpdate rru : toPush.values()) { - final TrackingRefUpdate tru = rru.getTrackingRefUpdate(); - if (tru != null) - res.add(tru); + walker.release(); } - return res; } private Map prepareRemoteUpdates() diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index 2475b9acfe..6b0a9b6cff 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -574,6 +574,7 @@ public class ReceivePack { service(); } finally { + walk.release(); try { if (pckOut != null) pckOut.flush(); 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 7e7d4c8927..02ce251bea 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -296,6 +296,7 @@ public class UploadPack { pckOut = new PacketLineOut(rawOut); service(); } finally { + walk.release(); if (timer != null) { try { timer.terminate(); @@ -567,7 +568,7 @@ public class UploadPack { SideBandOutputStream.CH_PROGRESS, bufsz, rawOut)); } - final PackWriter pw = new PackWriter(db); + final PackWriter pw = new PackWriter(db, walk.getObjectReader()); try { pw.setDeltaBaseAsOffset(options.contains(OPTION_OFS_DELTA)); pw.setThin(thin); -- 2.39.5