From b8c8008115e0c48d3d844af1dc858c700a9428e4 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 27 May 2015 09:40:32 -0700 Subject: [PATCH] FS: Extract GobblerThread into a private static class The primary goal is to improve exception readability. Since this is a standalone thread, just logging the stack trace of the caught exception is not very useful: java.io.IOException: Stream closed at java.io.BufferedInputStream.getBufIfOpen(BufferedInputStream.java:162) at java.io.BufferedInputStream.read(BufferedInputStream.java:258) at org.eclipse.jgit.util.FS$2.run(FS.java:451) Providing a named class eliminates the "FS$2", and including the command name provides a little more context in the error message. A future improvement might include the stack trace that created the GobblerThread as well. Change-Id: Ibf16d15b47a85b6f41844a177e398c2fc94f27b0 --- .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../src/org/eclipse/jgit/util/FS.java | 90 ++++++++++++------- 3 files changed, 59 insertions(+), 33 deletions(-) diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index f591eddfa..a2f06b1a2 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -257,6 +257,7 @@ exceptionCaughtDuringExecutionOfResetCommand=Exception caught during execution o exceptionCaughtDuringExecutionOfRevertCommand=Exception caught during execution of revert command. {0} exceptionCaughtDuringExecutionOfRmCommand=Exception caught during execution of rm command exceptionCaughtDuringExecutionOfTagCommand=Exception caught during execution of tag command +exceptionCaughtDuringExcecutionOfCommand=Exception caught during execution of command {0} in {1} exceptionHookExecutionInterrupted=Execution of "{0}" hook interrupted. exceptionOccurredDuringAddingOfOptionToALogCommand=Exception occurred during adding of {0} as option to a Log command exceptionOccurredDuringReadingOfGIT_DIR=Exception occurred during reading of $GIT_DIR/{0}. {1} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index b53c7c95f..9f6efef57 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -316,6 +316,7 @@ public static JGitText get() { /***/ public String exceptionCaughtDuringExecutionOfRevertCommand; /***/ public String exceptionCaughtDuringExecutionOfRmCommand; /***/ public String exceptionCaughtDuringExecutionOfTagCommand; + /***/ public String exceptionCaughtDuringExcecutionOfCommand; /***/ public String exceptionHookExecutionInterrupted; /***/ public String exceptionOccurredDuringAddingOfOptionToALogCommand; /***/ public String exceptionOccurredDuringReadingOfGIT_DIR; 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 fa0292e79..12dfe96b0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java @@ -440,40 +440,11 @@ protected static String readPipe(File dir, String[] command, String encoding, Ma if (env != null) { pb.environment().putAll(env); } - final Process p = pb.start(); - final BufferedReader lineRead = new BufferedReader( + Process p = pb.start(); + BufferedReader lineRead = new BufferedReader( new InputStreamReader(p.getInputStream(), encoding)); p.getOutputStream().close(); - final AtomicBoolean gooblerFail = new AtomicBoolean(false); - Thread gobbler = new Thread() { - public void run() { - InputStream is = p.getErrorStream(); - try { - int ch; - if (debug) - while ((ch = is.read()) != -1) - System.err.print((char) ch); - else - while (is.read() != -1) { - // ignore - } - } catch (IOException e) { - // Just print on stderr for debugging - if (debug) - e.printStackTrace(System.err); - gooblerFail.set(true); - } - try { - is.close(); - } catch (IOException e) { - // Just print on stderr for debugging - if (debug) { - LOG.debug("Caught exception in gobbler thread", e); //$NON-NLS-1$ - } - gooblerFail.set(true); - } - } - }; + GobblerThread gobbler = new GobblerThread(p, command, dir); gobbler.start(); String r = null; try { @@ -498,7 +469,7 @@ public void run() { int rc = p.waitFor(); gobbler.join(); if (rc == 0 && r != null && r.length() > 0 - && !gooblerFail.get()) + && !gobbler.fail.get()) return r; if (debug) { LOG.debug("readpipe rc=" + rc); //$NON-NLS-1$ @@ -517,6 +488,59 @@ public void run() { return null; } + private static class GobblerThread extends Thread { + private final Process p; + private final String desc; + private final String dir; + private final boolean debug = LOG.isDebugEnabled(); + private final AtomicBoolean fail = new AtomicBoolean(); + + private GobblerThread(Process p, String[] command, File dir) { + this.p = p; + if (debug) { + this.desc = Arrays.asList(command).toString(); + this.dir = dir.toString(); + } else { + this.desc = null; + this.dir = null; + } + } + + public void run() { + InputStream is = p.getErrorStream(); + try { + int ch; + if (debug) { + while ((ch = is.read()) != -1) { + System.err.print((char) ch); + } + } else { + while (is.read() != -1) { + // ignore + } + } + } catch (IOException e) { + logError(e); + fail.set(true); + } + try { + is.close(); + } catch (IOException e) { + logError(e); + fail.set(true); + } + } + + private void logError(Throwable t) { + if (!debug) { + return; + } + String msg = MessageFormat.format( + JGitText.get().exceptionCaughtDuringExcecutionOfCommand, desc, dir); + LOG.debug(msg, t); + } + } + /** * @return the path to the Git executable or {@code null} if it cannot be * determined.