Cleanup stream usage WRT filters

As it is right now some streams leak out of the filter construct. This
change clarifies responsibilities and fixes stream leaks

Change-Id: Ib9717d43a701a06a502434d64214d13a392de5ab
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

2 files changed, 45 insertions, 30 deletions

diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/CleanFilter.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/CleanFilter.java
index fccbae7955..217e46f48e 100644
--- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/CleanFilter.java
+++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/CleanFilter.java
@@ -167,6 +167,7 @@ public class CleanFilter extends FilterCommand {
 				}
 				LfsPointer lfsPointer = new LfsPointer(loid, size);
 				lfsPointer.encode(out);
+				in.close();
 				out.close();
 				return -1;
 			}
@@ -174,6 +175,7 @@ public class CleanFilter extends FilterCommand {
 			if (aOut != null) {
 				aOut.abort();
 			}
+			in.close();
 			out.close();
 			throw e;
 		}
diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/SmudgeFilter.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/SmudgeFilter.java
index 142e74df6a..5c0d3b47a7 100644
--- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/SmudgeFilter.java
+++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/SmudgeFilter.java
@@ -116,23 +116,29 @@ public class SmudgeFilter extends FilterCommand {
 	 * @param db
 	 *            a {@link org.eclipse.jgit.lib.Repository} object.
 	 * @param in
-	 *            a {@link java.io.InputStream} object.
+	 *            a {@link java.io.InputStream} object. The stream is closed in
+	 *            any case.
 	 * @param out
 	 *            a {@link java.io.OutputStream} object.
 	 * @throws java.io.IOException
+	 *             in case of an error
 	 */
 	public SmudgeFilter(Repository db, InputStream in, OutputStream out)
 			throws IOException {
 		super(in, out);
-		Lfs lfs = new Lfs(db);
-		LfsPointer res = LfsPointer.parseLfsPointer(in);
-		if (res != null) {
-			AnyLongObjectId oid = res.getOid();
-			Path mediaFile = lfs.getMediaFile(oid);
-			if (!Files.exists(mediaFile)) {
-				downloadLfsResource(lfs, db, res);
+		try {
+			Lfs lfs = new Lfs(db);
+			LfsPointer res = LfsPointer.parseLfsPointer(in);
+			if (res != null) {
+				AnyLongObjectId oid = res.getOid();
+				Path mediaFile = lfs.getMediaFile(oid);
+				if (!Files.exists(mediaFile)) {
+					downloadLfsResource(lfs, db, res);
+				}
+				this.in = Files.newInputStream(mediaFile);
 			}
-			this.in = Files.newInputStream(mediaFile);
+		} finally {
+			in.close(); // make sure the swapped stream is closed properly.
 		}
 	}
 
@@ -147,6 +153,7 @@ public class SmudgeFilter extends FilterCommand {
 	 *            the objects to download
 	 * @return the paths of all mediafiles which have been downloaded
 	 * @throws IOException
+	 * @since 4.11
 	 */
 	public static Collection<Path> downloadLfsResource(Lfs lfs, Repository db,
 			LfsPointer... res) throws IOException {
@@ -228,33 +235,39 @@ public class SmudgeFilter extends FilterCommand {
 	/** {@inheritDoc} */
 	@Override
 	public int run() throws IOException {
-		int totalRead = 0;
-		int length = 0;
-		if (in != null) {
-			byte[] buf = new byte[8192];
-			while ((length = in.read(buf)) != -1) {
-				out.write(buf, 0, length);
-				totalRead += length;
+		try {
+			int totalRead = 0;
+			int length = 0;
+			if (in != null) {
+				byte[] buf = new byte[8192];
+				while ((length = in.read(buf)) != -1) {
+					out.write(buf, 0, length);
+					totalRead += length;
 
-				// when threshold reached, loop back to the caller. otherwise we
-				// could only support files up to 2GB (int return type)
-				// properly. we will be called again as long as we don't return
-				// -1 here.
-				if (totalRead >= MAX_COPY_BYTES) {
-					// leave streams open - we need them in the next call.
-					return totalRead;
+					// when threshold reached, loop back to the caller.
+					// otherwise we could only support files up to 2GB (int
+					// return type) properly. we will be called again as long as
+					// we don't return -1 here.
+					if (totalRead >= MAX_COPY_BYTES) {
+						// leave streams open - we need them in the next call.
+						return totalRead;
+					}
 				}
 			}
-		}
 
-		if (totalRead == 0 && length == -1) {
-			// we're totally done :)
-			in.close();
+			if (totalRead == 0 && length == -1) {
+				// we're totally done :) cleanup all streams
+				in.close();
+				out.close();
+				return length;
+			}
+
+			return totalRead;
+		} catch (IOException e) {
+			in.close(); // clean up - we swapped this stream.
 			out.close();
-			return length;
+			throw e;
 		}
-
-		return totalRead;
 	}
 
 }