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

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 dcd7970cbc7cd558c32a621b06a582cb4368009b..2f570ee51ca8dadb744b2e9a7b34fba3281c6f84 100644 (file)
--- 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;
                }
        }
 }