diff options
author | Klaus Aehlig <aehlig@google.com> | 2016-08-02 09:59:25 +0000 |
---|---|---|
committer | Philipp Wollermann <philwo@google.com> | 2016-08-02 11:16:29 +0000 |
commit | fbe4221dca7715edd784c2fc598877fb102cb8a3 (patch) | |
tree | c402e40014d02c24b359998f8b045663d609c5fb /src/main/java/com/google/devtools/build/lib | |
parent | 2a8d73109f5e539fc20e956e8139dc1bb5b65111 (diff) |
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
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java | 30 |
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(); } |