SDK/ADT: Refactor grabProcessOutput.

We have many implementations of grabProcessOutput.
This introduces an utility in sdklib that takes
an interface to capture out/err and uses this in
various places instead of duplicating the code.

Change-Id: I7e6eea443c592ae5670485369233e1cd002ff9cc

4 files changed, 201 insertions, 267 deletions

diff --git a/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/avd/AvdManager.java b/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/avd/AvdManager.java
index f6610c0..436f2e8 100644
--- a/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/avd/AvdManager.java
+++ b/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/avd/AvdManager.java
@@ -26,9 +26,10 @@ import com.android.sdklib.SdkConstants;
 import com.android.sdklib.SdkManager;
 import com.android.sdklib.internal.avd.AvdInfo.AvdStatus;
 import com.android.sdklib.internal.project.ProjectProperties;
+import com.android.sdklib.util.GrabProcessOutput;
+import com.android.sdklib.util.GrabProcessOutput.IProcessOutput;
 import com.android.util.Pair;
 
-import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
@@ -36,7 +37,6 @@ import java.io.FileOutputStream;
 import java.io.FileWriter;
 import java.io.FilenameFilter;
 import java.io.IOException;
-import java.io.InputStreamReader;
 import java.io.OutputStreamWriter;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -1393,10 +1393,27 @@ public class AvdManager {
             command[2] = location;
             Process process = Runtime.getRuntime().exec(command);
 
-            ArrayList<String> errorOutput = new ArrayList<String>();
-            ArrayList<String> stdOutput = new ArrayList<String>();
-            int status = grabProcessOutput(process, errorOutput, stdOutput,
-                    true /* waitForReaders */);
+            final ArrayList<String> errorOutput = new ArrayList<String>();
+            final ArrayList<String> stdOutput = new ArrayList<String>();
+
+            int status = GrabProcessOutput.grabProcessOutput(
+                    process,
+                    true /*waitForReaders*/,
+                    new IProcessOutput() {
+                        @Override
+                        public void out(String line) {
+                            if (line != null) {
+                                stdOutput.add(line);
+                            }
+                        }
+
+                        @Override
+                        public void err(String line) {
+                            if (line != null) {
+                                errorOutput.add(line);
+                            }
+                        }
+                    });
 
             if (status == 0) {
                 return true;
@@ -1417,89 +1434,6 @@ public class AvdManager {
     }
 
     /**
-     * Gets the stderr/stdout outputs of a process and returns when the process is done.
-     * Both <b>must</b> be read or the process will block on windows.
-     * @param process The process to get the ouput from
-     * @param errorOutput The array to store the stderr output. cannot be null.
-     * @param stdOutput The array to store the stdout output. cannot be null.
-     * @param waitforReaders if true, this will wait for the reader threads.
-     * @return the process return code.
-     * @throws InterruptedException
-     */
-    private int grabProcessOutput(final Process process, final ArrayList<String> errorOutput,
-            final ArrayList<String> stdOutput, boolean waitforReaders)
-            throws InterruptedException {
-        assert errorOutput != null;
-        assert stdOutput != null;
-        // read the lines as they come. if null is returned, it's
-        // because the process finished
-        Thread t1 = new Thread("") { //$NON-NLS-1$
-            @Override
-            public void run() {
-                // create a buffer to read the stderr output
-                InputStreamReader is = new InputStreamReader(process.getErrorStream());
-                BufferedReader errReader = new BufferedReader(is);
-
-                try {
-                    while (true) {
-                        String line = errReader.readLine();
-                        if (line != null) {
-                            errorOutput.add(line);
-                        } else {
-                            break;
-                        }
-                    }
-                } catch (IOException e) {
-                    // do nothing.
-                }
-            }
-        };
-
-        Thread t2 = new Thread("") { //$NON-NLS-1$
-            @Override
-            public void run() {
-                InputStreamReader is = new InputStreamReader(process.getInputStream());
-                BufferedReader outReader = new BufferedReader(is);
-
-                try {
-                    while (true) {
-                        String line = outReader.readLine();
-                        if (line != null) {
-                            stdOutput.add(line);
-                        } else {
-                            break;
-                        }
-                    }
-                } catch (IOException e) {
-                    // do nothing.
-                }
-            }
-        };
-
-        t1.start();
-        t2.start();
-
-        // it looks like on windows process#waitFor() can return
-        // before the thread have filled the arrays, so we wait for both threads and the
-        // process itself.
-        if (waitforReaders) {
-            try {
-                t1.join();
-            } catch (InterruptedException e) {
-                // nothing to do here
-            }
-            try {
-                t2.join();
-            } catch (InterruptedException e) {
-                // nothing to do here
-            }
-        }
-
-        // get the return code from the process
-        return process.waitFor();
-    }
-
-    /**
      * Removes an {@link AvdInfo} from the internal list.
      *
      * @param avdInfo The {@link AvdInfo} to remove.
diff --git a/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/build/KeystoreHelper.java b/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/build/KeystoreHelper.java
index d769510..06f5351 100644
--- a/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/build/KeystoreHelper.java
+++ b/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/build/KeystoreHelper.java
@@ -18,11 +18,11 @@ package com.android.sdklib.internal.build;
 
 import com.android.sdklib.internal.build.DebugKeyProvider.IKeyGenOutput;
 import com.android.sdklib.internal.build.DebugKeyProvider.KeytoolException;
+import com.android.sdklib.util.GrabProcessOutput;
+import com.android.sdklib.util.GrabProcessOutput.IProcessOutput;
 
-import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
-import java.io.InputStreamReader;
 import java.security.KeyStoreException;
 import java.security.NoSuchAlgorithmException;
 import java.security.UnrecoverableEntryException;
@@ -56,7 +56,7 @@ public final class KeystoreHelper {
             String keyPassword,
             String description,
             int validityYears,
-            IKeyGenOutput output)
+            final IKeyGenOutput output)
             throws KeyStoreException, NoSuchAlgorithmException, CertificateException,
             UnrecoverableEntryException, IOException, KeytoolException {
 
@@ -104,9 +104,35 @@ public final class KeystoreHelper {
         // launch the command line process
         int result = 0;
         try {
-            result = grabProcessOutput(Runtime.getRuntime().exec(commandArray), output);
+            Process process = Runtime.getRuntime().exec(commandArray);
+            result = GrabProcessOutput.grabProcessOutput(
+                    process,
+                    true /*waitForReaders*/,
+                    new IProcessOutput() {
+                        @Override
+                        public void out(String line) {
+                            if (line != null) {
+                                if (output != null) {
+                                    output.out(line);
+                                } else {
+                                    System.out.println(line);
+                                }
+                            }
+                        }
+
+                        @Override
+                        public void err(String line) {
+                            if (line != null) {
+                                if (output != null) {
+                                    output.err(line);
+                                } else {
+                                    System.err.println(line);
+                                }
+                            }
+                        }
+                    });
         } catch (Exception e) {
-            // create the command line as one string
+            // create the command line as one string for debugging purposes
             StringBuilder builder = new StringBuilder();
             boolean firstArg = true;
             for (String arg : commandArray) {
@@ -139,89 +165,4 @@ public final class KeystoreHelper {
 
         return true;
     }
-
-    /**
-     * Get the stderr/stdout outputs of a process and return when the process is done.
-     * Both <b>must</b> be read or the process will block on windows.
-     * @param process The process to get the ouput from
-     * @return the process return code.
-     */
-    private static int grabProcessOutput(final Process process, final IKeyGenOutput output) {
-        // read the lines as they come. if null is returned, it's
-        // because the process finished
-        Thread t1 = new Thread("") {
-            @Override
-            public void run() {
-                // create a buffer to read the stderr output
-                InputStreamReader is = new InputStreamReader(process.getErrorStream());
-                BufferedReader errReader = new BufferedReader(is);
-
-                try {
-                    while (true) {
-                        String line = errReader.readLine();
-                        if (line != null) {
-                            if (output != null) {
-                                output.err(line);
-                            } else {
-                                System.err.println(line);
-                            }
-                        } else {
-                            break;
-                        }
-                    }
-                } catch (IOException e) {
-                    // do nothing.
-                }
-            }
-        };
-
-        Thread t2 = new Thread("") {
-            @Override
-            public void run() {
-                InputStreamReader is = new InputStreamReader(process.getInputStream());
-                BufferedReader outReader = new BufferedReader(is);
-
-                try {
-                    while (true) {
-                        String line = outReader.readLine();
-                        if (line != null) {
-                            if (output != null) {
-                                output.out(line);
-                            } else {
-                                System.out.println(line);
-                            }
-                        } else {
-                            break;
-                        }
-                    }
-                } catch (IOException e) {
-                    // do nothing.
-                }
-            }
-        };
-
-        t1.start();
-        t2.start();
-
-        // it looks like on windows process#waitFor() can return
-        // before the thread have filled the arrays, so we wait for both threads and the
-        // process itself.
-        try {
-            t1.join();
-        } catch (InterruptedException e) {
-        }
-        try {
-            t2.join();
-        } catch (InterruptedException e) {
-        }
-
-        // get the return code from the process
-        try {
-            return process.waitFor();
-        } catch (InterruptedException e) {
-            // since we're waiting for the output thread above, we should never actually wait
-            // on the process to end, since it'll be done by the time we call waitFor()
-            return 0;
-        }
-    }
 }
diff --git a/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/repository/ToolPackage.java b/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/repository/ToolPackage.java
index ee7265f..ffd561f 100755
--- a/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/repository/ToolPackage.java
+++ b/sdkmanager/libs/sdklib/src/com/android/sdklib/internal/repository/ToolPackage.java
@@ -23,13 +23,12 @@ import com.android.sdklib.SdkManager;
 import com.android.sdklib.internal.repository.Archive.Arch;

 import com.android.sdklib.internal.repository.Archive.Os;

 import com.android.sdklib.repository.SdkRepoConstants;

+import com.android.sdklib.util.GrabProcessOutput;

+import com.android.sdklib.util.GrabProcessOutput.IProcessOutput;

 

 import org.w3c.dom.Node;

 

-import java.io.BufferedReader;

 import java.io.File;

-import java.io.IOException;

-import java.io.InputStreamReader;

 import java.util.Map;

 import java.util.Properties;

 import java.util.regex.Matcher;

@@ -240,7 +239,7 @@ public class ToolPackage extends Package implements IMinPlatformToolsDependency
      * {@inheritDoc}

      */

     @Override

-    public void postInstallHook(Archive archive, ITaskMonitor monitor, File installFolder) {

+    public void postInstallHook(Archive archive, final ITaskMonitor monitor, File installFolder) {

         super.postInstallHook(archive, monitor, installFolder);

 

         if (installFolder == null) {

@@ -266,16 +265,33 @@ public class ToolPackage extends Package implements IMinPlatformToolsDependency
             return;

         }

 

-        Process proc;

         int status = -1;

 

         try {

-            proc = Runtime.getRuntime().exec(

+            Process proc = Runtime.getRuntime().exec(

                     shell + scriptName, // command

                     null,       // environment

                     libDir);    // working dir

 

-            status = grabProcessOutput(proc, monitor, scriptName);

+            final String tag = scriptName;

+            status = GrabProcessOutput.grabProcessOutput(

+                    proc,

+                    false /*waitForReaders*/,

+                    new IProcessOutput() {

+                        @Override

+                        public void out(String line) {

+                            if (line != null) {

+                                monitor.log("[%1$s] %2$s", tag, line);

+                            }

+                        }

+

+                        @Override

+                        public void err(String line) {

+                            if (line != null) {

+                                monitor.logError("[%1$s] Error: %2$s", tag, line);

+                            }

+                        }

+                    });

 

         } catch (Exception e) {

             monitor.logError("Exception: %s", e.toString());

@@ -287,87 +303,6 @@ public class ToolPackage extends Package implements IMinPlatformToolsDependency
         }

     }

 

-    /**

-     * Gets the stderr/stdout outputs of a process and returns when the process is done.

-     * Both <b>must</b> be read or the process will block on windows.

-     * @param process The process to get the ouput from.

-     * @param monitor The monitor where to output errors.

-     * @param scriptName The name of script being executed.

-     * @return the process return code.

-     * @throws InterruptedException

-     */

-    private int grabProcessOutput(final Process process,

-            final ITaskMonitor monitor,

-            final String scriptName)

-                throws InterruptedException {

-        // read the lines as they come. if null is returned, it's

-        // because the process finished

-        Thread t1 = new Thread("") { //$NON-NLS-1$

-            @Override

-            public void run() {

-                // create a buffer to read the stderr output

-                InputStreamReader is = new InputStreamReader(process.getErrorStream());

-                BufferedReader errReader = new BufferedReader(is);

-

-                try {

-                    while (true) {

-                        String line = errReader.readLine();

-                        if (line != null) {

-                            monitor.logError("[%1$s] Error: %2$s", scriptName, line);

-                        } else {

-                            break;

-                        }

-                    }

-                } catch (IOException e) {

-                    // do nothing.

-                }

-            }

-        };

-

-        Thread t2 = new Thread("") { //$NON-NLS-1$

-            @Override

-            public void run() {

-                InputStreamReader is = new InputStreamReader(process.getInputStream());

-                BufferedReader outReader = new BufferedReader(is);

-

-                try {

-                    while (true) {

-                        String line = outReader.readLine();

-                        if (line != null) {

-                            monitor.log("[%1$s] %2$s", scriptName, line);

-                        } else {

-                            break;

-                        }

-                    }

-                } catch (IOException e) {

-                    // do nothing.

-                }

-            }

-        };

-

-        t1.start();

-        t2.start();

-

-        // it looks like on windows process#waitFor() can return

-        // before the thread have filled the arrays, so we wait for both threads and the

-        // process itself.

-        /* Disabled since not used. Do we really need this?

-        if (waitforReaders) {

-            try {

-                t1.join();

-            } catch (InterruptedException e) {

-            }

-            try {

-                t2.join();

-            } catch (InterruptedException e) {

-            }

-        }

-        */

-

-        // get the return code from the process

-        return process.waitFor();

-    }

-

     @Override

     public int hashCode() {

         final int prime = 31;

diff --git a/sdkmanager/libs/sdklib/src/com/android/sdklib/util/GrabProcessOutput.java b/sdkmanager/libs/sdklib/src/com/android/sdklib/util/GrabProcessOutput.java
new file mode 100755
index 0000000..b470153
--- /dev/null
+++ b/sdkmanager/libs/sdklib/src/com/android/sdklib/util/GrabProcessOutput.java
@@ -0,0 +1,124 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.sdklib.util;
+
+import com.android.annotations.NonNull;
+import com.android.annotations.Nullable;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+
+public class GrabProcessOutput {
+
+    public interface IProcessOutput {
+        /**
+         * Processes an stdout message line.
+         * @param line The stdout message line. Null when the reader reached the end of stdout.
+         */
+        public void out(@Nullable String line);
+        /**
+         * Processes an stderr message line.
+         * @param line The stderr message line. Null when the reader reached the end of stderr.
+         */
+        public void err(@Nullable String line);
+    }
+
+    /**
+     * Get the stderr/stdout outputs of a process and return when the process is done.
+     * Both <b>must</b> be read or the process will block on windows.
+     *
+     * @param process The process to get the ouput from.
+     * @param output Optional object to capture stdout/stderr.
+     *      Note that on Windows capturing the output is not optional. If output is null
+     *      the stdout/stderr will be captured and discarded.
+     * @param waitForReaders True to wait for the reader threads to finish.
+     * @return the process return code.
+     * @throws InterruptedException if {@link Process#waitFor()} was interrupted.
+     */
+    public static int grabProcessOutput(
+            @NonNull final Process process,
+            boolean waitForReaders,
+            @Nullable final IProcessOutput output) throws InterruptedException {
+        // read the lines as they come. if null is returned, it's
+        // because the process finished
+        Thread threadErr = new Thread("stderr") {
+            @Override
+            public void run() {
+                // create a buffer to read the stderr output
+                InputStreamReader is = new InputStreamReader(process.getErrorStream());
+                BufferedReader errReader = new BufferedReader(is);
+
+                try {
+                    while (true) {
+                        String line = errReader.readLine();
+                        if (output != null) {
+                            output.err(line);
+                        }
+                        if (line == null) {
+                            break;
+                        }
+                    }
+                } catch (IOException e) {
+                    // do nothing.
+                }
+            }
+        };
+
+        Thread threadOut = new Thread("stdout") {
+            @Override
+            public void run() {
+                InputStreamReader is = new InputStreamReader(process.getInputStream());
+                BufferedReader outReader = new BufferedReader(is);
+
+                try {
+                    while (true) {
+                        String line = outReader.readLine();
+                        if (output != null) {
+                            output.out(line);
+                        }
+                        if (line == null) {
+                            break;
+                        }
+                    }
+                } catch (IOException e) {
+                    // do nothing.
+                }
+            }
+        };
+
+        threadErr.start();
+        threadOut.start();
+
+        // it looks like on windows process#waitFor() can return
+        // before the thread have filled the arrays, so we wait for both threads and the
+        // process itself.
+        if (waitForReaders) {
+            try {
+                threadErr.join();
+            } catch (InterruptedException e) {
+            }
+            try {
+                threadOut.join();
+            } catch (InterruptedException e) {
+            }
+        }
+
+        // get the return code from the process
+        return process.waitFor();
+    }
+}