From 76b4ed6a8503ea3f9514d931bf97e85910c319f5 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 15 Mar 2018 13:44:00 +0900 Subject: [PATCH] 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 --- .../src/org/eclipse/jgit/util/FS.java | 26 +++++++++++-------- 1 file 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 edcb9d7a6..a2756e845 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 int runProcess(ProcessBuilder processBuilder, 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) {