CollectStdOutExecutor and CollectStdOutStdErrExecutor were plain wrong, were losing data and were blocking compilation. Fixes #3124 and #3115

arduino-core/src/cc/arduino/MyStreamPumper.java

@@ -0,0 +1,78 @@
+package cc.arduino;
+
+import processing.app.debug.MessageConsumer;
+
+import java.io.BufferedReader;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+
+/**
+ * A version of StreamPumper from commons-exec that write to a MessageConsumer
+ */
+public class MyStreamPumper implements Runnable {
+
+  /**
+   * the input stream to pump from
+   */
+  private final BufferedReader reader;
+
+  /**
+   * the output stream to pmp into
+   */
+  private final MessageConsumer messageConsumer;
+
+  /**
+   * was the end of the stream reached
+   */
+  private boolean finished;
+
+  public MyStreamPumper(final InputStream is, final MessageConsumer messageConsumer) {
+    this.reader = new BufferedReader(new InputStreamReader(is));
+    this.messageConsumer = messageConsumer;
+  }
+
+  /**
+   * Copies data from the input stream to the output stream. Terminates as
+   * soon as the input stream is closed or an error occurs.
+   */
+  public void run() {
+    synchronized (this) {
+      // Just in case this object is reused in the future
+      finished = false;
+    }
+
+    try {
+      String line;
+      while ((line = reader.readLine()) != null) {
+        messageConsumer.message(line + "\n");
+      }
+    } catch (Exception e) {
+      // nothing to do - happens quite often with watchdog
+    } finally {
+      synchronized (this) {
+        finished = true;
+        notifyAll();
+      }
+    }
+  }
+
+  /**
+   * Tells whether the end of the stream has been reached.
+   *
+   * @return true is the stream has been exhausted.
+   */
+  public synchronized boolean isFinished() {
+    return finished;
+  }
+
+  /**
+   * This method blocks until the stream pumper finishes.
+   *
+   * @see #isFinished()
+   */
+  public synchronized void waitFor() throws InterruptedException {
+    while (!isFinished()) {
+      wait();
+    }
+  }
+}

arduino-core/src/cc/arduino/contributions/packages/ContributionInstaller.java

@@ -37,14 +37,15 @@ import cc.arduino.utils.MultiStepProgress;
 import cc.arduino.utils.Progress;
 import com.google.common.collect.Collections2;
 import org.apache.commons.exec.CommandLine;
+import org.apache.commons.exec.DefaultExecutor;
 import org.apache.commons.exec.Executor;
+import org.apache.commons.exec.PumpStreamHandler;
 import processing.app.BaseNoGui;
 import processing.app.I18n;
 import processing.app.Platform;
 import processing.app.PreferencesData;
 import processing.app.helpers.FileUtils;
 import processing.app.helpers.filefilters.OnlyDirs;
-import processing.app.tools.CollectStdOutStdErrExecutor;
 
 import java.io.ByteArrayOutputStream;
 import java.io.File;
@@ -179,7 +180,8 @@ public class ContributionInstaller {
 
     ByteArrayOutputStream stdout = new ByteArrayOutputStream();
     ByteArrayOutputStream stderr = new ByteArrayOutputStream();
-    Executor executor = new CollectStdOutStdErrExecutor(stdout, stderr);
+    Executor executor = new DefaultExecutor();
+    executor.setStreamHandler(new PumpStreamHandler(stdout, stderr));
     executor.setWorkingDirectory(folder);
     executor.setExitValues(null);
     int exitValue = executor.execute(new CommandLine(postInstallScript));

arduino-core/src/processing/app/debug/Compiler.java

@@ -35,14 +35,13 @@ import java.util.Map;
 import java.util.SortedSet;
 import java.util.TreeSet;
 
+import cc.arduino.MyStreamPumper;
 import cc.arduino.packages.BoardPort;
 import cc.arduino.packages.Uploader;
 import cc.arduino.packages.UploaderFactory;
 
 import org.apache.commons.compress.utils.IOUtils;
-import org.apache.commons.exec.CommandLine;
+import org.apache.commons.exec.*;
-import org.apache.commons.exec.DefaultExecutor;
-import org.apache.commons.exec.ExecuteStreamHandler;
 import processing.app.BaseNoGui;
 import processing.app.I18n;
 import processing.app.PreferencesData;
@@ -703,37 +702,13 @@ public class Compiler implements MessageConsumer {
     }
 
     DefaultExecutor executor = new DefaultExecutor();
-    executor.setStreamHandler(new ExecuteStreamHandler() {
+    executor.setStreamHandler(new PumpStreamHandler() {
-      @Override
-      public void setProcessInputStream(OutputStream os) throws IOException {
-
-      }
 
       @Override
-      public void setProcessErrorStream(InputStream is) throws IOException {
+      protected Thread createPump(InputStream is, OutputStream os, boolean closeWhenExhausted) {
-        forwardToMessage(is);
+        final Thread result = new Thread(new MyStreamPumper(is, Compiler.this));
-      }
+        result.setDaemon(true);
-
+        return result;
-      @Override
-      public void setProcessOutputStream(InputStream is) throws IOException {
-        forwardToMessage(is);
-      }
-
-      private void forwardToMessage(InputStream is) throws IOException {
-        BufferedReader reader = new BufferedReader(new InputStreamReader(is));
-        String line;
-        while ((line = reader.readLine()) != null) {
-          message(line + "\n");
-        }
-      }
-
-      @Override
-      public void start() throws IOException {
-
-      }
-
-      @Override
-      public void stop() {
 
       }
     });

arduino-core/src/processing/app/linux/Platform.java

@@ -23,11 +23,12 @@
 package processing.app.linux;
 
 import org.apache.commons.exec.CommandLine;
+import org.apache.commons.exec.DefaultExecutor;
 import org.apache.commons.exec.Executor;
+import org.apache.commons.exec.PumpStreamHandler;
 import processing.app.PreferencesData;
 import processing.app.debug.TargetPackage;
 import processing.app.legacy.PConstants;
-import processing.app.tools.CollectStdOutExecutor;
 
 import java.io.ByteArrayOutputStream;
 import java.io.File;
@@ -124,7 +125,8 @@ public class Platform extends processing.app.Platform {
   public Map<String, Object> resolveDeviceAttachedTo(String serial, Map<String, TargetPackage> packages, String devicesListOutput) {
     assert packages != null;
     ByteArrayOutputStream baos = new ByteArrayOutputStream();
-    Executor executor = new CollectStdOutExecutor(baos);
+    Executor executor = new DefaultExecutor();
+    executor.setStreamHandler(new PumpStreamHandler(baos, null));
 
     try {
       CommandLine toDevicePath = CommandLine.parse("udevadm info -q path -n " + serial);

arduino-core/src/processing/app/macosx/Platform.java

@@ -25,12 +25,13 @@ package processing.app.macosx;
 import cc.arduino.packages.BoardPort;
 import com.apple.eio.FileManager;
 import org.apache.commons.exec.CommandLine;
+import org.apache.commons.exec.DefaultExecutor;
 import org.apache.commons.exec.Executor;
+import org.apache.commons.exec.PumpStreamHandler;
 import org.apache.commons.lang3.StringUtils;
 import processing.app.debug.TargetPackage;
 import processing.app.legacy.PApplet;
 import processing.app.legacy.PConstants;
-import processing.app.tools.CollectStdOutExecutor;
 
 import java.awt.*;
 import java.io.*;
@@ -67,7 +68,8 @@ public class Platform extends processing.app.Platform {
   private void discoverRealOsArch() throws IOException {
     CommandLine uname = CommandLine.parse("uname -m");
     ByteArrayOutputStream baos = new ByteArrayOutputStream();
-    CollectStdOutExecutor executor = new CollectStdOutExecutor(baos);
+    Executor executor = new DefaultExecutor();
+    executor.setStreamHandler(new PumpStreamHandler(baos, null));
     executor.execute(uname);
     osArch = StringUtils.trim(new String(baos.toByteArray()));
   }
@@ -214,7 +216,8 @@ public class Platform extends processing.app.Platform {
   @Override
   public String preListAllCandidateDevices() {
     ByteArrayOutputStream baos = new ByteArrayOutputStream();
-    Executor executor = new CollectStdOutExecutor(baos);
+    Executor executor = new DefaultExecutor();
+    executor.setStreamHandler(new PumpStreamHandler(baos, null));
 
     try {
       CommandLine toDevicePath = CommandLine.parse("/usr/sbin/system_profiler SPUSBDataType");

arduino-core/src/processing/app/tools/CollectStdOutExecutor.java

@@ -1,44 +0,0 @@
-package processing.app.tools;
-
-import org.apache.commons.exec.DefaultExecutor;
-import org.apache.commons.exec.ExecuteStreamHandler;
-
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
-
-/**
- * Handy process executor, collecting stdout into a given OutputStream
- */
-public class CollectStdOutExecutor extends DefaultExecutor {
-
-  public CollectStdOutExecutor(final OutputStream stdout) {
-    this.setStreamHandler(new ExecuteStreamHandler() {
-      @Override
-      public void setProcessInputStream(OutputStream outputStream) throws IOException {
-      }
-
-      @Override
-      public void setProcessErrorStream(InputStream inputStream) throws IOException {
-      }
-
-      @Override
-      public void setProcessOutputStream(InputStream inputStream) throws IOException {
-        byte[] buf = new byte[4096];
-        int bytes = -1;
-        while ((bytes = inputStream.read(buf)) != -1) {
-          stdout.write(buf, 0, bytes);
-        }
-      }
-
-      @Override
-      public void start() throws IOException {
-      }
-
-      @Override
-      public void stop() {
-      }
-    });
-
-  }
-}

arduino-core/src/processing/app/tools/CollectStdOutStdErrExecutor.java

@@ -1,49 +0,0 @@
-package processing.app.tools;
-
-import org.apache.commons.exec.DefaultExecutor;
-import org.apache.commons.exec.ExecuteStreamHandler;
-
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
-
-/**
- * Handy process executor, collecting stdout and stderr into given OutputStreams
- */
-public class CollectStdOutStdErrExecutor extends DefaultExecutor {
-
-  public CollectStdOutStdErrExecutor(final OutputStream stdout, final OutputStream stderr) {
-    this.setStreamHandler(new ExecuteStreamHandler() {
-      @Override
-      public void setProcessInputStream(OutputStream outputStream) throws IOException {
-      }
-
-      @Override
-      public void setProcessErrorStream(InputStream inputStream) throws IOException {
-        byte[] buf = new byte[4096];
-        int bytes = -1;
-        while ((bytes = inputStream.read(buf)) != -1) {
-          stderr.write(buf, 0, bytes);
-        }
-      }
-
-      @Override
-      public void setProcessOutputStream(InputStream inputStream) throws IOException {
-        byte[] buf = new byte[4096];
-        int bytes = -1;
-        while ((bytes = inputStream.read(buf)) != -1) {
-          stdout.write(buf, 0, bytes);
-        }
-      }
-
-      @Override
-      public void start() throws IOException {
-      }
-
-      @Override
-      public void stop() {
-      }
-    });
-
-  }
-}

arduino-core/src/processing/app/windows/Platform.java

@@ -23,11 +23,12 @@
 package processing.app.windows;
 
 import org.apache.commons.exec.CommandLine;
+import org.apache.commons.exec.DefaultExecutor;
 import org.apache.commons.exec.Executor;
+import org.apache.commons.exec.PumpStreamHandler;
 import processing.app.debug.TargetPackage;
 import processing.app.legacy.PApplet;
 import processing.app.legacy.PConstants;
-import processing.app.tools.CollectStdOutExecutor;
 
 import java.io.ByteArrayOutputStream;
 import java.io.File;
@@ -62,7 +63,8 @@ public class Platform extends processing.app.Platform {
 
   private String getFolderPathFromRegistry(String folderType) throws IOException {
     ByteArrayOutputStream baos = new ByteArrayOutputStream();
-    Executor executor = new CollectStdOutExecutor(baos);
+    Executor executor = new DefaultExecutor();
+    executor.setStreamHandler(new PumpStreamHandler(baos, null));
 
     CommandLine toDevicePath = CommandLine.parse("reg query \"HKCU\\Software\\Microsoft\\Windows\\CurrentVersion\\Explorer\\Shell Folders\" /v \"" + folderType + "\"");
     executor.execute(toDevicePath);
@@ -193,7 +195,8 @@ public class Platform extends processing.app.Platform {
   @Override
   public String preListAllCandidateDevices() {
     ByteArrayOutputStream baos = new ByteArrayOutputStream();
-    Executor executor = new CollectStdOutExecutor(baos);
+    Executor executor = new DefaultExecutor();
+    executor.setStreamHandler(new PumpStreamHandler(baos, null));
 
     try {
       String listComPorts = new File(System.getProperty("user.dir"), "hardware/tools/listComPorts.exe").getCanonicalPath();