ExperimentalEventHandler: narrow scope of synchronized blocks

...to in particular never own a lock while waiting for the
update thread to finish. This avoids a deadlock when the update
thread tries to enter the synchronized part of doRefresh while
the thread trying to stop the update thread holds the lock.

Also renable experimental_ui_test now that the race condition leading
to the deadlock is fixed. The absence of flakiness has been verified
by running the test locally 100 times. Fixes #1560.

--
Change-Id: I5d85b347e6cb7a1da8d5a724d6f9cd7461e33e5b
Reviewed-on: https://bazel-review.googlesource.com/#/c/4225
MOS_MIGRATED_REVID=129079398

1 files changed, 18 insertions, 12 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 16609cac3a..1ba9523665 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
@@ -309,31 +309,37 @@ public class ExperimentalEventHandler implements EventHandler {
   }
 
   @Subscribe
-  public synchronized void buildComplete(BuildCompleteEvent event) {
+  public void buildComplete(BuildCompleteEvent event) {
     // The final progress bar will flow into the scroll-back buffer, to if treat
     // it as an event and add a time stamp, if events are supposed to have a time stmap.
-    if (showTimestamp) {
-      stateTracker.buildComplete(event, TIMESTAMP_FORMAT.print(clock.currentTimeMillis()));
-    } else {
-      stateTracker.buildComplete(event);
+    synchronized (this) {
+      if (showTimestamp) {
+        stateTracker.buildComplete(event, TIMESTAMP_FORMAT.print(clock.currentTimeMillis()));
+      } else {
+        stateTracker.buildComplete(event);
+      }
+      ignoreRefreshLimitOnce();
+      refresh();
+      buildComplete = true;
     }
-    ignoreRefreshLimitOnce();
-    refresh();
-    buildComplete = true;
     stopUpdateThread();
     flushStdOutStdErrBuffers();
   }
 
   @Subscribe
-  public synchronized void noBuild(NoBuildEvent event) {
-    buildComplete = true;
+  public void noBuild(NoBuildEvent event) {
+    synchronized (this) {
+      buildComplete = true;
+    }
     stopUpdateThread();
     flushStdOutStdErrBuffers();
   }
 
   @Subscribe
-  public synchronized void afterCommand(AfterCommandEvent event) {
-    buildComplete = true;
+  public void afterCommand(AfterCommandEvent event) {
+    synchronized (this) {
+      buildComplete = true;
+    }
     stopUpdateThread();
   }