Skip to content

Commit

Permalink
ExperimentalEventHandler: add a shutdown flag
Browse files Browse the repository at this point in the history
    We've observed hangs of Bazel on shutdown with thread stacks indicating
    that the cli-update-thread does not exit properly. We don't know why
    this is happening, and have no easy way to reproduce.

    My best guess is that the interrupt is swallowed somewhere, although I
    was unable to pinpoint a specific place where that could happen. As a
    defensive mechanism, this change adds a boolean flag that signals to the
    CLI thread to shutdown in case the interrupt signal gets lost. This also
    helps with a potential race between startUpdateThread and
    stopUpdateThread.

    Possible fix for #8432.

    PiperOrigin-RevId: 249612495
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent f70fde6 commit 0d1328d
Showing 1 changed file with 23 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.google.devtools.build.lib.actions.RunningActionEvent;
import com.google.devtools.build.lib.actions.ScanningActionEvent;
import com.google.devtools.build.lib.actions.SchedulingActionEvent;
import com.google.devtools.build.lib.actions.StoppedScanningActionEvent;
import com.google.devtools.build.lib.analysis.AnalysisPhaseCompleteEvent;
import com.google.devtools.build.lib.analysis.NoBuildEvent;
import com.google.devtools.build.lib.analysis.NoBuildRequestFinishedEvent;
Expand Down Expand Up @@ -68,8 +67,6 @@
import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.logging.Logger;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -117,7 +114,6 @@ public class ExperimentalEventHandler implements EventHandler {
private boolean progressBarNeedsRefresh;
private volatile boolean shutdown;
private final AtomicReference<Thread> updateThread;
private final Lock updateLock;
private byte[] stdoutBuffer;
private byte[] stderrBuffer;
private final Set<String> messagesSeen;
Expand Down Expand Up @@ -285,7 +281,6 @@ public ExperimentalEventHandler(
this.stderrBuffer = new byte[] {};
this.dateShown = false;
this.updateThread = new AtomicReference<>();
this.updateLock = new ReentrantLock();
// The progress bar has not been updated yet.
ignoreRefreshLimitOnce();
}
Expand Down Expand Up @@ -780,13 +775,6 @@ public void scanningAction(ScanningActionEvent event) {
refresh();
}

@Subscribe
@AllowConcurrentEvents
public void stopScanningAction(StoppedScanningActionEvent event) {
stateTracker.stopScanningAction(event);
refresh();
}

@Subscribe
@AllowConcurrentEvents
public void schedulingAction(SchedulingActionEvent event) {
Expand Down Expand Up @@ -915,39 +903,35 @@ private void doRefresh(boolean fromUpdateThread) {
}
long nowMillis = clock.currentTimeMillis();
if (lastRefreshMillis + minimalDelayMillis < nowMillis) {
if (updateLock.tryLock()) {
synchronized (this) {
try {
synchronized (this) {
if (showProgress && (progressBarNeedsRefresh || timeBasedRefresh())) {
progressBarNeedsRefresh = false;
clearProgressBar();
addProgressBar();
terminal.flush();
double remaining = remainingCapacity();
if (remaining < CAPACITY_INCREASE_UPDATE_DELAY) {
// Increase the update interval if the start producing too much output
minimalDelayMillis = Math.max(minimalDelayMillis, 1000);
if (remaining < CAPACITY_UPDATE_DELAY_5_SECONDS) {
minimalDelayMillis = Math.max(minimalDelayMillis, 5000);
}
}
if (!cursorControl || remaining < CAPACITY_UPDATE_DELAY_AS_NO_CURSES) {
// If we can't update the progress bar in place, make sure we increase the update
// interval as time progresses, to avoid too many progress messages in place.
minimalDelayMillis =
Math.max(
minimalDelayMillis,
Math.round(
NO_CURSES_MINIMAL_RELATIVE_PROGRESS_RATE_LMIT
* (clock.currentTimeMillis() - uiStartTimeMillis)));
minimalUpdateInterval = Math.max(minimalDelayMillis, MAXIMAL_UPDATE_DELAY_MILLIS);
if (showProgress && (progressBarNeedsRefresh || timeBasedRefresh())) {
progressBarNeedsRefresh = false;
clearProgressBar();
addProgressBar();
terminal.flush();
double remaining = remainingCapacity();
if (remaining < CAPACITY_INCREASE_UPDATE_DELAY) {
// Increase the update interval if the start producing too much output
minimalDelayMillis = Math.max(minimalDelayMillis, 1000);
if (remaining < CAPACITY_UPDATE_DELAY_5_SECONDS) {
minimalDelayMillis = Math.max(minimalDelayMillis, 5000);
}
}
if (!cursorControl || remaining < CAPACITY_UPDATE_DELAY_AS_NO_CURSES) {
// If we can't update the progress bar in place, make sure we increase the update
// interval as time progresses, to avoid too many progress messages in place.
minimalDelayMillis =
Math.max(
minimalDelayMillis,
Math.round(
NO_CURSES_MINIMAL_RELATIVE_PROGRESS_RATE_LMIT
* (clock.currentTimeMillis() - uiStartTimeMillis)));
minimalUpdateInterval = Math.max(minimalDelayMillis, MAXIMAL_UPDATE_DELAY_MILLIS);
}
}
} catch (IOException e) {
logger.warning("IO Error writing to output stream: " + e);
} finally {
updateLock.unlock();
}
}
} else {
Expand Down

0 comments on commit 0d1328d

Please sign in to comment.