Change StreamGobbler to Runnable to avoid unused Future

It can be considered a programming error to create a Future<T>
but do nothing with that object. There is an async computation
happening and without holding and checking the Future for done
or exception the caller has no idea if it has completed.

FS doesn't really care about these StreamGobblers finishing.
Instead use Runnable with execute(Runnable), which doesn't
return a Future.

Change-Id: I93b66d1f6c869e66be5c1169d8edafe781e601f6

1 files changed, 15 insertions, 12 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
index dcd7970cbc..2f570ee51c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
@@ -59,7 +59,6 @@ import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
-import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
@@ -1011,16 +1010,13 @@ public abstract class FS {
 		IOException ioException = null;
 		try {
 			process = processBuilder.start();
-			final Callable<Void> errorGobbler = new StreamGobbler(
-					process.getErrorStream(), errRedirect);
-			final Callable<Void> outputGobbler = new StreamGobbler(
-					process.getInputStream(), outRedirect);
-			executor.submit(errorGobbler);
-			executor.submit(outputGobbler);
+			executor.execute(
+					new StreamGobbler(process.getErrorStream(), errRedirect));
+			executor.execute(
+					new StreamGobbler(process.getInputStream(), outRedirect));
 			OutputStream outputStream = process.getOutputStream();
 			if (inRedirect != null) {
-				new StreamGobbler(inRedirect, outputStream)
-						.call();
+				new StreamGobbler(inRedirect, outputStream).copy();
 			}
 			try {
 				outputStream.close();
@@ -1336,7 +1332,7 @@ public abstract class FS {
 	 * streams.
 	 * </p>
 	 */
-	private static class StreamGobbler implements Callable<Void> {
+	private static class StreamGobbler implements Runnable {
 		private InputStream in;
 
 		private OutputStream out;
@@ -1346,7 +1342,15 @@ public abstract class FS {
 			this.out = output;
 		}
 
-		public Void call() throws IOException {
+		public void run() {
+			try {
+				copy();
+			} catch (IOException e) {
+				// Do nothing on read failure; leave streams open.
+			}
+		}
+
+		void copy() throws IOException {
 			boolean writeFailure = false;
 			byte buffer[] = new byte[4096];
 			int readBytes;
@@ -1363,7 +1367,6 @@ public abstract class FS {
 					}
 				}
 			}
-			return null;
 		}
 	}
 }