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

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 f591eddfa33778c039357accc4561d129ecbfcd2..a2f06b1a2b477178f49b29b62c764f1db1e19147 100644 (file)
--- 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 b53c7c95f02af83e35c80d80cb1c41d5c2f1e64b..9f6efef570eb65c80f2f8fae95163a144735fb68 100644 (file)
--- 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 class JGitText extends TranslationBundle {
        /***/ 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 fa0292e7937b62ede812e8129da52fc557eb0743..12dfe96b05a3ac4270d562166a44ec8d6500fd2d 100644 (file)
--- 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 @@ public abstract class FS {
                        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 abstract class FS {
                                        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 abstract class FS {
                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.