Remove RunUtil.RunnableResult closeStreamAfterRun argument

closeStreamAfterRun's behavior is currently slightly wrong - in that if
either stdout or stderr streams are provided but not both, the other
stream that RunnableResult creates is never closed. However, it's
unnecessary in any case; it's better to just track in RunnableResult
whether it created the stream(s).

Bug: 123529934
Bug: None
Test: m -j tradefed-all &&
tools/tradefederation/core/tests/run_tradefed_aosp_presubmit.sh

Change-Id: Ic48d9d17af4a6abfe56d9fb11152b7c26cee656c
Merged-In: Ic48d9d17af4a6abfe56d9fb11152b7c26cee656c
diff --git a/src/com/android/tradefed/util/RunUtil.java b/src/com/android/tradefed/util/RunUtil.java
index 78483f8..8737a0c 100644
--- a/src/com/android/tradefed/util/RunUtil.java
+++ b/src/com/android/tradefed/util/RunUtil.java
@@ -128,7 +128,7 @@
      */
     @Override
     public CommandResult runTimedCmd(final long timeout, final String... command) {
-        return runTimedCmd(timeout, null, null, true, command);
+        return runTimedCmd(timeout, null, null, command);
     }
 
     /**
@@ -137,19 +137,8 @@
     @Override
     public CommandResult runTimedCmd(final long timeout, OutputStream stdout,
             OutputStream stderr, final String... command) {
-        return runTimedCmd(timeout, stdout, stderr, false, command);
-    }
-
-    /**
-     * Helper method to do a runTimeCmd call with or without outputStream specified.
-     *
-     * @return a {@CommandResult} containing results from command
-     */
-    private CommandResult runTimedCmd(final long timeout, OutputStream stdout,
-            OutputStream stderr, boolean closeStreamAfterRun, final String... command) {
         final CommandResult result = new CommandResult();
-        IRunUtil.IRunnableResult osRunnable =
-                createRunnableResult(result, stdout, stderr, closeStreamAfterRun, command);
+        IRunUtil.IRunnableResult osRunnable = createRunnableResult(result, stdout, stderr, command);
         CommandStatus status = runTimed(timeout, osRunnable, true);
         result.setStatus(status);
         return result;
@@ -164,10 +153,8 @@
             CommandResult result,
             OutputStream stdout,
             OutputStream stderr,
-            boolean closeStreamAfterRun,
             String... command) {
-        return new RunnableResult(
-                result, null, createProcessBuilder(command), stdout, stderr, closeStreamAfterRun);
+        return new RunnableResult(result, null, createProcessBuilder(command), stdout, stderr);
     }
 
     /** {@inheritDoc} */
@@ -545,27 +532,27 @@
         private Thread mExecutionThread;
         private OutputStream stdOut = null;
         private OutputStream stdErr = null;
-        private final boolean mCloseStreamAfterRun;
+        private boolean mCreatedStdoutStream = false;
+        private boolean mCreatedStderrStream = false;
         private final Object mLock = new Object();
         private boolean mCancelled = false;
 
         RunnableResult(final CommandResult result, final String input,
                 final ProcessBuilder processBuilder) {
-            this(result, input, processBuilder, null, null, false);
-            stdOut = new ByteArrayOutputStream();
-            stdErr = new ByteArrayOutputStream();
+            this(result, input, processBuilder, null, null);
         }
 
         /**
-         * Alternative constructor that allows redirecting the output to any Outputstream.
-         * Stdout and stderr can be independently redirected to different Outputstream
-         * implementations.
-         * If streams are null, default behavior of using a buffer will be used.
+         * Alternative constructor that allows redirecting the output to any Outputstream. Stdout
+         * and stderr can be independently redirected to different Outputstream implementations. If
+         * streams are null, default behavior of using a buffer will be used.
          */
-        RunnableResult(final CommandResult result, final String input,
-                final ProcessBuilder processBuilder, OutputStream stdoutStream,
-                OutputStream stderrStream, boolean closeStreamAfterRun) {
-            mCloseStreamAfterRun = closeStreamAfterRun;
+        RunnableResult(
+                final CommandResult result,
+                final String input,
+                final ProcessBuilder processBuilder,
+                OutputStream stdoutStream,
+                OutputStream stderrStream) {
             mProcessBuilder = processBuilder;
             mInput = input;
             mCommandResult = result;
@@ -579,11 +566,13 @@
                 stdOut = stdoutStream;
             } else {
                 stdOut = new ByteArrayOutputStream();
+                mCreatedStdoutStream = true;
             }
             if (stderrStream != null) {
                 stdErr = stderrStream;
             } else {
                 stdErr = new ByteArrayOutputStream();
+                mCreatedStderrStream = true;
             }
         }
 
@@ -624,6 +613,15 @@
                                 mProcess.getErrorStream(),
                                 stdErr,
                                 String.format("inheritio-stderr-%s", mProcessBuilder.command()));
+
+                // Close the stdout/err streams if created by us. Streams provided by the caller
+                // should be closed by the caller.
+                if (mCreatedStdoutStream) {
+                    stdOut.close();
+                }
+                if (mCreatedStderrStream) {
+                    stdErr.close();
+                }
             }
             // Wait for process to complete.
             int rc = Integer.MIN_VALUE;
@@ -639,12 +637,6 @@
                     if (stderrThread.isAlive()) {
                         CLog.d("stderr read thread %s still alive.", stderrThread.toString());
                     }
-                    // close the buffer that holds stdout/err content if default stream
-                    // stream specified by caller should be handled by the caller.
-                    if (mCloseStreamAfterRun) {
-                        stdOut.close();
-                        stdErr.close();
-                    }
                 } finally {
                     // Write out the streams to the result.
                     if (stdOut instanceof ByteArrayOutputStream) {
diff --git a/tests/src/com/android/tradefed/util/RunUtilTest.java b/tests/src/com/android/tradefed/util/RunUtilTest.java
index a6cc003..2fe1336 100644
--- a/tests/src/com/android/tradefed/util/RunUtilTest.java
+++ b/tests/src/com/android/tradefed/util/RunUtilTest.java
@@ -75,11 +75,8 @@
                 CommandResult result,
                 OutputStream stdout,
                 OutputStream stderr,
-                boolean closeStreamAfterRun,
                 String... command) {
-            IRunnableResult real =
-                    super.createRunnableResult(
-                            result, stdout, stderr, closeStreamAfterRun, command);
+            IRunnableResult real = super.createRunnableResult(result, stdout, stderr, command);
             mMockRunnableResult = (RunnableResult) Mockito.spy(real);
             try {
                 if (mShouldThrow) {