Automated g4 rollback of commit 48c636637a5a4fba1c0c509bdc3724450155d792.

*** Reason for rollback ***

Rollforward with fix

To have stdout/stderr in the BuildEventStream, the BuildEventStreamerModule
registers a listener that accumulates the values for the streamer to
collect it and regularly send it over the stream. As we have to also catch
stdout/stderr that happens early in the build, we have to register the listener
before(!) the options are parsed; therefore it is registered unconditionally.
Now, if there is no streamer created after parsing the options there is no
one to collect the data and it grows indefinitely. Fix this, by disabling
the collection in this case.

*** Original change description ***

Automated g4 rollback of commit 9e37b2e52d6e42eec15712942c7f208b64c651e5.

*** Reason for rollback ***

Results in NegativeArraySizeExceptions when there's a high volume of data.

*** Original change description ***

BEP: Report stdout/stderr

By recording registering a properly synchronized OutErr as listener
and providing it as OutErrProvider to the BuildEventStreamer.

Change-Id: Id553fcdb85327be28561634268511304fcc2ad3f
PiperOrigin-RevId: 155374710

2 files changed, 121 insertions, 0 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamerModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamerModule.java
index 846c8aa302..9fa92186fb 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamerModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamerModule.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.runtime;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.devtools.build.lib.buildeventstream.transports.BuildEventTransportFactory.createFromOptions;
+import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
@@ -29,10 +30,12 @@ import com.google.devtools.build.lib.buildeventstream.PathConverter;
 import com.google.devtools.build.lib.buildeventstream.transports.BuildEventStreamOptions;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.ExitCode;
+import com.google.devtools.build.lib.util.io.OutErr;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsProvider;
 import java.io.IOException;
+import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -57,6 +60,72 @@ public class BuildEventStreamerModule extends BlazeModule {
 
   private BuildEventRecorder buildEventRecorder;
 
+  /**
+   * {@link OutputStream} suitably synchonized for producer-consumer use cases.
+   * The method {@link #readAndReset()} allows to read the bytes accumulated so far
+   * and simultaneously truncate precisely the bytes read. Moreover, upon such a reset
+   * the amount of memory retained is reset to a small constant. This is a difference
+   * with resecpt to the behaviour of the standard classes {@link ByteArrayOutputStream}
+   * which only resets the index but keeps the array. This difference matters, as we need
+   * to support output peeks without retaining this ammount of memory for the rest of the
+   * build.
+   */
+  private static class SynchronizedOutputStream extends OutputStream {
+
+    private byte[] buf;
+    private int count;
+    private boolean discardAll;
+
+    SynchronizedOutputStream() {
+      buf = new byte[64];
+      count = 0;
+      discardAll = false;
+    }
+
+    public synchronized void setDiscardAll() {
+      discardAll = true;
+      count = 0;
+      buf = null;
+    }
+
+    @Override
+    public synchronized void write(int oneByte) throws IOException {
+      if (discardAll) {
+        return;
+      }
+      if (count == buf.length) {
+        byte[] newbuf = new byte[count * 2 + 1];
+        System.arraycopy(buf, 0, newbuf, 0, count);
+        buf = newbuf;
+      }
+      buf[count++] = (byte) oneByte;
+    }
+
+    /**
+     * Read the contents of the stream and simultaneously clear them. Also, reset the amount of
+     * memory retained to a constant amount.
+     */
+    synchronized String readAndReset() {
+      String content = new String(buf, 0, count, UTF_8);
+      buf = new byte[64];
+      count = 0;
+      return content;
+    }
+
+    // While technically not needed, it is still a better user experience to have a write
+    // enter the stream in one go.
+    @Override
+    public synchronized void write(byte[] buffer, int offset, int count) throws IOException {
+      if (discardAll) {
+        return;
+      }
+      super.write(buffer, offset, count);
+    }
+  }
+
+  private SynchronizedOutputStream out;
+  private SynchronizedOutputStream err;
+
   @Override
   public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
     return ImmutableList.<Class<? extends OptionsBase>>of(BuildEventStreamOptions.class);
@@ -70,6 +139,13 @@ public class BuildEventStreamerModule extends BlazeModule {
   }
 
   @Override
+  public OutErr getOutputListener() {
+    this.out = new SynchronizedOutputStream();
+    this.err = new SynchronizedOutputStream();
+    return OutErr.create(this.out, this.err);
+  }
+
+  @Override
   public void handleOptions(OptionsProvider optionsProvider) {
     checkState(commandEnvironment != null, "Methods called out of order");
     Optional<BuildEventStreamer> maybeStreamer =
@@ -81,9 +157,38 @@ public class BuildEventStreamerModule extends BlazeModule {
       for (BuildEvent event : buildEventRecorder.getEvents()) {
         streamer.buildEvent(event);
       }
+      final SynchronizedOutputStream theOut = this.out;
+      final SynchronizedOutputStream theErr = this.err;
+      // out and err should be non-null at this point, as getOutputListener is supposed to
+      // be always called before handleOptions. But let's still prefer a stream with no
+      // stdout/stderr over an aborted build.
+      streamer.registerOutErrProvider(
+          new BuildEventStreamer.OutErrProvider() {
+            @Override
+            public String getOut() {
+              if (theOut == null) {
+                return null;
+              }
+              return theOut.readAndReset();
+            }
+
+            @Override
+            public String getErr() {
+              if (theErr == null) {
+                return null;
+              }
+              return theErr.readAndReset();
+            }
+          });
+    } else {
+      // If there is no streamer to consume the output, we should not try to accumulate it.
+      this.out.setDiscardAll();
+      this.err.setDiscardAll();
     }
     commandEnvironment.getEventBus().unregister(buildEventRecorder);
     this.buildEventRecorder = null;
+    this.out = null;
+    this.err = null;
   }
 
   @VisibleForTesting
diff --git a/src/test/shell/integration/build_event_stream_test.sh b/src/test/shell/integration/build_event_stream_test.sh
index 32c128612f..f6cd382b84 100755
--- a/src/test/shell/integration/build_event_stream_test.sh
+++ b/src/test/shell/integration/build_event_stream_test.sh
@@ -384,4 +384,20 @@ function test_loading_failure_keep_going() {
 #   expect_not_log 'aborted'
 # }
 
+function test_stdout_stderr_reported() {
+  # Verify that bazel's stdout/stderr is included in the build event stream.
+
+  # Make sure we generate enough output on stderr
+  bazel clean --expunge
+  bazel test --experimental_build_event_text_file=$TEST_log --curses=no \
+        pkg:slow 2>stderr.log || fail "slowtest failed"
+  # Take a line that is likely not the output of an action (possibly reported
+  # independently in the stream) and still characteristic enough to not occur
+  # in the stream by accident. Taking the first line mentioning the test name
+  # is likely some form of progress report.
+  sample_line=`cat stderr.log | grep 'slow' | head -1 | tr '[]' '..'`
+  echo "Sample regexp of stderr: ${sample_line}"
+  expect_log "stderr.*$sample_line"
+}
+
 run_suite "Integration tests for the build event stream"