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 <spearce@spearce.org>

3 files changed, 39 insertions, 24 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<String, Ref> refs = db.getAllRefs();
-		refs.remove(Constants.HEAD);
-		adv.send(refs);
-		out.close();
+			Map<String, Ref> 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();
+			}
 		}
 	}