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

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 0513fb2e243f5ec4a68556c88059b5095389350b..9aa09b4648f9a4bb2a4adb1861e411f82a54a63f 100644 (file)
--- 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 @@ class Archive extends TextBuiltin {
                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 bbd9c8d826dc49824597115006bf2d4dcc099bb9..11c5702885f687be92cc519463780cd5dd380532 100644 (file)
--- 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 @@ import org.eclipse.jgit.treewalk.TreeWalk;
  *
  * <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 @@ import org.eclipse.jgit.treewalk.TreeWalk;
  * <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 @@ public class ArchiveCommand extends GitCommand<OutputStream> {
        }
 
        private OutputStream out;
-       private TreeWalk walk;
+       private ObjectId tree;
        private String format = "tar";
 
        /**
@@ -205,27 +206,20 @@ public class ArchiveCommand extends GitCommand<OutputStream> {
         */
        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 @@ public class ArchiveCommand extends GitCommand<OutputStream> {
                        } 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 @@ public class ArchiveCommand extends GitCommand<OutputStream> {
         */
        @Override
        public OutputStream call() throws GitAPIException {
+               checkCallable();
+
                final Format<?> fmt = lookupFormat(format);
                return writeArchive(fmt);
        }
@@ -264,11 +261,13 @@ public class ArchiveCommand extends GitCommand<OutputStream> {
         * @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;
        }