From 131b09106f76cdfca1d7292f2cc39a35834c7a67 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Fri, 20 Jan 2017 08:41:29 -0800 Subject: [PATCH] Change StreamGobbler to Runnable to avoid unused Future It can be considered a programming error to create a Future 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 --- .../src/org/eclipse/jgit/util/FS.java | 27 ++++++++++--------- 1 file 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 dcd7970cb..2f570ee51 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 errorGobbler = new StreamGobbler( - process.getErrorStream(), errRedirect); - final Callable 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. *

*/ - private static class StreamGobbler implements Callable { + 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; } } }