From f99fa9d23e93fdd34124e2100d629680894477d7 Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrn@google.com>
Date: Fri, 24 May 2013 18:24:53 -0700
Subject: [PATCH] Release ArchiveCommand's ObjectReader in call()

Make call() release all private resources so instead of using a
pattern like

	ArchiveCommand cmd = git.archive();
	try {
		cmd.setTree(tree)
			. ...
			.call();
	} finally {
		cmd.release();
	}

callers can just use git.archive().setTree(tree)....call() directly.

This involves pushing more work out of parameter setters and into
call() so the ObjectReader is not allocated and potentially leaked
before then.

Change-Id: I699f703c6302696e1cc276d7ab8ee597d82f2c5d
---
 .../src/org/eclipse/jgit/pgm/Archive.java     | 11 ++--
 .../org/eclipse/jgit/api/ArchiveCommand.java  | 57 +++++++++----------
 2 files changed, 33 insertions(+), 35 deletions(-)

diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Archive.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Archive.java
index 0513fb2e2..9aa09b464 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Archive.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Archive.java
@@ -72,15 +72,14 @@ protected void run() throws Exception {
 		if (tree == null)
 			throw die(CLIText.get().treeIsRequired);
 
-		final ArchiveCommand cmd = new Git(db).archive();
 		try {
-			cmd.setTree(tree)
-					.setFormat(format)
-					.setOutputStream(outs).call();
+			new Git(db).archive()
+				.setTree(tree)
+				.setFormat(format)
+				.setOutputStream(outs)
+				.call();
 		} catch (GitAPIException e) {
 			throw die(e.getMessage());
-		} finally {
-			cmd.release();
 		}
 	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/ArchiveCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/ArchiveCommand.java
index bbd9c8d82..11c570288 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/ArchiveCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/ArchiveCommand.java
@@ -72,12 +72,12 @@
  *
  * <pre>
  * ArchiveCommand.registerFormat("tar", new TarFormat());
- * cmd = git.archive();
  * try {
- *	cmd.setTree(db.resolve(&quot;HEAD&quot;))
- *		.setOutputStream(out).call();
+ *	git.archive()
+ *		.setTree(db.resolve(&quot;HEAD&quot;))
+ *		.setOutputStream(out)
+ *		.call();
  * } finally {
- *	cmd.release();
  *	ArchiveCommand.unregisterFormat("tar");
  * }
  * </pre>
@@ -87,11 +87,12 @@
  * <pre>
  * ArchiveCommand.registerFormat("zip", new ZipFormat());
  * try {
- *	cmd.setTree(db.resolve(&quot;master&quot;))
+ *	git.archive().
+ *		.setTree(db.resolve(&quot;master&quot;))
  *		.setFormat("zip")
- *		.setOutputStream(out).call();
+ *		.setOutputStream(out)
+ *		.call();
  * } finally {
- *	cmd.release();
  *	ArchiveCommand.unregisterFormat("zip");
  * }
  * </pre>
@@ -197,7 +198,7 @@ private static Format<?> lookupFormat(String formatName) throws UnsupportedForma
 	}
 
 	private OutputStream out;
-	private TreeWalk walk;
+	private ObjectId tree;
 	private String format = "tar";
 
 	/**
@@ -205,27 +206,20 @@ private static Format<?> lookupFormat(String formatName) throws UnsupportedForma
 	 */
 	public ArchiveCommand(Repository repo) {
 		super(repo);
-		walk = new TreeWalk(repo);
-	}
-
-	/**
-	 * Release any resources used by the internal ObjectReader.
-	 * <p>
-	 * This does not close the output stream set with setOutputStream, which
-	 * belongs to the caller.
-	 */
-	public void release() {
-		walk.release();
+		setCallable(false);
 	}
 
 	private <T extends Closeable>
 	OutputStream writeArchive(Format<T> fmt) throws GitAPIException {
-		final MutableObjectId idBuf = new MutableObjectId();
-		final T outa = fmt.createArchiveOutputStream(out);
-		final ObjectReader reader = walk.getObjectReader();
-
+		final TreeWalk walk = new TreeWalk(repo);
 		try {
+			final T outa = fmt.createArchiveOutputStream(out);
 			try {
+				final MutableObjectId idBuf = new MutableObjectId();
+				final ObjectReader reader = walk.getObjectReader();
+				final RevWalk rw = new RevWalk(walk.getObjectReader());
+
+				walk.reset(rw.parseTree(tree));
 				walk.setRecursive(true);
 				while (walk.next()) {
 					final String name = walk.getPathString();
@@ -242,13 +236,14 @@ OutputStream writeArchive(Format<T> fmt) throws GitAPIException {
 			} finally {
 				outa.close();
 			}
+			return out;
 		} catch (IOException e) {
 			// TODO(jrn): Throw finer-grained errors.
 			throw new JGitInternalException(
 					JGitText.get().exceptionCaughtDuringExecutionOfArchiveCommand, e);
+		} finally {
+			walk.release();
 		}
-
-		return out;
 	}
 
 	/**
@@ -256,6 +251,8 @@ OutputStream writeArchive(Format<T> fmt) throws GitAPIException {
 	 */
 	@Override
 	public OutputStream call() throws GitAPIException {
+		checkCallable();
+
 		final Format<?> fmt = lookupFormat(format);
 		return writeArchive(fmt);
 	}
@@ -264,11 +261,13 @@ public OutputStream call() throws GitAPIException {
 	 * @param tree
 	 *            the tag, commit, or tree object to produce an archive for
 	 * @return this
-	 * @throws IOException
 	 */
-	public ArchiveCommand setTree(ObjectId tree) throws IOException {
-		final RevWalk rw = new RevWalk(walk.getObjectReader());
-		walk.reset(rw.parseTree(tree));
+	public ArchiveCommand setTree(ObjectId tree) {
+		if (tree == null)
+			throw new IllegalArgumentException();
+
+		this.tree = tree;
+		setCallable(true);
 		return this;
 	}