ExperimentalEventHandler: avoid recursive starting of the updateThread

Break the situation where an update thread tries to start another
update thread early, by telling the doRefresh method when it is
called from within an update thread. While no second thread would be
started in this case anyway (as the global variables indicate that
a thread is already in existence), a lock is still required for this
step, which can cause all kinds of problems. Note that we don't have
to worry about the update thread being about to be stopped in that moment,
as only global events indicating completion stop the update thread.

--
Change-Id: Ib025ba2b12bcd339c590593213eeedfd9cb230d3
Reviewed-on: https://bazel-review.googlesource.com/#/c/4224
MOS_MIGRATED_REVID=129077891

1 files changed, 11 insertions, 7 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java
index dbc0dbffd5..16609cac3a 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java
@@ -39,15 +39,13 @@ import com.google.devtools.build.lib.util.io.LoggingTerminalWriter;
 import com.google.devtools.build.lib.util.io.OutErr;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus;
-
-import org.joda.time.format.DateTimeFormat;
-import org.joda.time.format.DateTimeFormatter;
-
 import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.logging.Logger;
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
 
 /** An experimental new output stream. */
 public class ExperimentalEventHandler implements EventHandler {
@@ -422,7 +420,7 @@ public class ExperimentalEventHandler implements EventHandler {
     }
   }
 
-  private void doRefresh() {
+  private void doRefresh(boolean fromUpdateThread) {
     if (buildComplete) {
       return;
     }
@@ -444,10 +442,16 @@ public class ExperimentalEventHandler implements EventHandler {
       // We skipped an update due to rate limiting. If this however, turned
       // out to be the last update for a long while, we need to show it in a
       // timely manner, as it best describes the current state.
-      startUpdateThread();
+      if (!fromUpdateThread) {
+        startUpdateThread();
+      }
     }
   }
 
+  private void doRefresh() {
+    doRefresh(false);
+  }
+
   private void refreshSoon() {
     // Schedule an update of the progress bar in the near future, unless there is already
     // a future update scheduled.
@@ -506,7 +510,7 @@ public class ExperimentalEventHandler implements EventHandler {
                             && mustRefreshAfterMillis < clock.currentTimeMillis()) {
                           progressBarNeedsRefresh = true;
                         }
-                        eventHandler.doRefresh();
+                        eventHandler.doRefresh(/* fromUpdateThread= */ true);
                       }
                     } catch (InterruptedException e) {
                       // Ignore