FS#runProcess: Fix OutputStream left unclosed after IOException

The runProcess method creates an OutputStream that is not managed by
a try-with-resource because it's manually closed and any IOException
raised by the close() method is explicitly ignored.

Suppress the resource warning with an explanatory comment.

Enclose the call to StreamGobbler#copy in an inner try-block, and move
the call to close() inside its finally block. This prevents the stream
from being left unclosed if StreamGobbler#copy raises IOException.

Change-Id: Idca9adfc4d87e0989d787ad8239c055c0c849814
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

1 files changed, 15 insertions, 11 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 edcb9d7a6a..a2756e845c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
@@ -1108,19 +1108,23 @@ public abstract class FS {
 					new StreamGobbler(process.getErrorStream(), errRedirect));
 			executor.execute(
 					new StreamGobbler(process.getInputStream(), outRedirect));
+			@SuppressWarnings("resource") // Closed in the finally block
 			OutputStream outputStream = process.getOutputStream();
-			if (inRedirect != null) {
-				new StreamGobbler(inRedirect, outputStream).copy();
-			}
 			try {
-				outputStream.close();
-			} catch (IOException e) {
-				// When the process exits before consuming the input, the OutputStream
-				// is replaced with the null output stream. This null output stream
-				// throws IOException for all write calls. When StreamGobbler fails to
-				// flush the buffer because of this, this close call tries to flush it
-				// again. This causes another IOException. Since we ignore the
-				// IOException in StreamGobbler, we also ignore the exception here.
+				if (inRedirect != null) {
+					new StreamGobbler(inRedirect, outputStream).copy();
+				}
+			} finally {
+				try {
+					outputStream.close();
+				} catch (IOException e) {
+					// When the process exits before consuming the input, the OutputStream
+					// is replaced with the null output stream. This null output stream
+					// throws IOException for all write calls. When StreamGobbler fails to
+					// flush the buffer because of this, this close call tries to flush it
+					// again. This causes another IOException. Since we ignore the
+					// IOException in StreamGobbler, we also ignore the exception here.
+				}
 			}
 			return process.waitFor();
 		} catch (IOException e) {