RefAdvertiser: Avoid object parsing

It isn't strictly necessary to validate every reference's target
object is reachable in the repository before advertising it to a
client. This is an expensive operation when there are thousands of
references, and its very unlikely that a reference uses a missing
object, because garbage collection proceeds from the references and
walks down through the graph. So trying to hide a dangling reference
from clients is relatively pointless.

Even if we are trying to avoid giving a client a corrupt repository,
this simple check isn't sufficient.  It is possible for a reference to
point to a valid commit, but that commit to have a missing blob in its
root tree.  This can be caused by staging a file into the index,
waiting several weeks, then committing that file while also racing
against a prune.  The prune may delete the blob, since its
modification time is more than 2 weeks ago, but retain the commit,
since its modification time is right now.

Such graph corruption is already caught during PackWriter as it
enumerates the graph from the client's want list and digs back
to the roots or common base.  Leave the reference validation also
for that same phase, where we know we have to parse the object to
support the enumeration.

Change-Id: Iee70ead0d3ed2d2fcc980417d09d7a69b05f5c2f
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

1 files changed, 21 insertions, 31 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 647919e065..ffaa13153c 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
@@ -56,8 +56,6 @@ import javax.servlet.http.HttpServletResponse;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevFlag;
-import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.transport.RefAdvertiser;
 import org.eclipse.jgit.util.HttpSupport;
 
@@ -73,36 +71,28 @@ class InfoRefsServlet extends HttpServlet {
 		rsp.setCharacterEncoding(Constants.CHARACTER_ENCODING);
 
 		final Repository db = getRepository(req);
-		final RevWalk walk = new RevWalk(db);
-		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(db);
+		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();
-		} finally {
-			walk.release();
-		}
+		Map<String, Ref> refs = db.getAllRefs();
+		refs.remove(Constants.HEAD);
+		adv.send(refs);
+		out.close();
 	}
 }