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
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 16609ca..1ba9523 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 @@
}
@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();
}
diff --git a/src/test/shell/integration/BUILD b/src/test/shell/integration/BUILD
index 9224c46..35958d4 100644
--- a/src/test/shell/integration/BUILD
+++ b/src/test/shell/integration/BUILD
@@ -36,7 +36,6 @@
srcs = ["experimental_ui_test.sh"],
data = [":test-deps"],
shard_count = 6,
- tags = ["manual"],
)
sh_test(
