ExperimentalEventHandler: narrow synchronization blocks

In particular, do not own a lock while waiting for another
thread---especially not the update thread that might be waiting
for the very lock were holding.

--
Change-Id: I46aef3585b712e25cd4317004eacd8b48557a341
Reviewed-on: https://bazel-review.googlesource.com/#/c/3462
MOS_MIGRATED_REVID=120686232

1 files changed, 46 insertions, 32 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 ee0e542ece..b6fc87d9c8 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
@@ -283,19 +283,21 @@ public class ExperimentalEventHandler extends BlazeCommandEventHandler {
     doRefresh();
   }
 
-  private synchronized void doRefresh() {
+  private void doRefresh() {
     long nowMillis = clock.currentTimeMillis();
     if (lastRefreshMillis + minimalDelayMillis < nowMillis) {
-      try {
-        if (progressBarNeedsRefresh || timeBasedRefresh()) {
-          progressBarNeedsRefresh = false;
-          lastRefreshMillis = nowMillis;
-          clearProgressBar();
-          addProgressBar();
-          terminal.flush();
+      synchronized (this) {
+        try {
+          if (progressBarNeedsRefresh || timeBasedRefresh()) {
+            progressBarNeedsRefresh = false;
+            lastRefreshMillis = nowMillis;
+            clearProgressBar();
+            addProgressBar();
+            terminal.flush();
+          }
+        } catch (IOException e) {
+          LOG.warning("IO Error writing to output stream: " + e);
         }
-      } catch (IOException e) {
-        LOG.warning("IO Error writing to output stream: " + e);
       }
       if (!stateTracker.progressBarTimeDependent()) {
         stopUpdateThread();
@@ -327,37 +329,49 @@ public class ExperimentalEventHandler extends BlazeCommandEventHandler {
     lastRefreshMillis = clock.currentTimeMillis() - minimalDelayMillis - 1;
   }
 
-  private synchronized void startUpdateThread() {
-    if (updateThread == null) {
-      final ExperimentalEventHandler eventHandler = this;
-      updateThread =
-          new Thread(
-              new Runnable() {
-                @Override
-                public void run() {
-                  try {
-                    while (true) {
-                      Thread.sleep(minimalUpdateInterval);
-                      eventHandler.doRefresh();
+  private void startUpdateThread() {
+    Thread threadToStart = null;
+    synchronized (this) {
+      if (updateThread == null) {
+        final ExperimentalEventHandler eventHandler = this;
+        updateThread =
+            new Thread(
+                new Runnable() {
+                  @Override
+                  public void run() {
+                    try {
+                      while (true) {
+                        Thread.sleep(minimalUpdateInterval);
+                        eventHandler.doRefresh();
+                      }
+                    } catch (InterruptedException e) {
+                      // Ignore
                     }
-                  } catch (InterruptedException e) {
-                    // Ignore
                   }
-                }
-              });
-      updateThread.start();
+                });
+        threadToStart = updateThread;
+      }
+      if (threadToStart != null) {
+        threadToStart.start();
+      }
     }
   }
 
-  private synchronized void stopUpdateThread() {
-    if (updateThread != null) {
-      updateThread.interrupt();
+  private void stopUpdateThread() {
+    Thread threadToWaitFor = null;
+    synchronized (this) {
+      if (updateThread != null) {
+        threadToWaitFor = updateThread;
+        updateThread = null;
+      }
+    }
+    if (threadToWaitFor != null) {
+      threadToWaitFor.interrupt();
       try {
-        updateThread.join();
+        threadToWaitFor.join();
       } catch (InterruptedException e) {
         // Ignore
       }
-      updateThread = null;
     }
   }