From 639f89d7682cadff723ac210fa37101f37762a9d Mon Sep 17 00:00:00 2001 From: Brandon Jacklyn Date: Tue, 21 Sep 2021 23:28:40 -0700 Subject: [PATCH] Fix [Prepa] actions stuck in active state Fixed #13985 UiStateTracker can process ActionProgress events after an ActionCompletion event has been fired. This has the effect of recreating the action object in the activeActions Map because Map.computeIfAbsent() is being used to retrieve the action. Therefore, a new method getActionStateIfPresent is created which will return the action if it exists, otherwise return null, and actionProgress() uses this method so as to not inadverntantly recreate actions after they have already completed and been removed from the Map. Closes #14020. PiperOrigin-RevId: 398165993 --- .../devtools/build/lib/runtime/UiStateTracker.java | 11 +++++++++-- .../build/lib/runtime/UiStateTrackerTest.java | 4 ++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java b/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java index 84ff8f1f0a1e17..ee6c0a57dcb202 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java @@ -530,6 +530,11 @@ private ActionState getActionState( return activeActions.computeIfAbsent(actionId, (key) -> new ActionState(action, nanoTimeNow)); } + @Nullable + private ActionState getActionStateIfPresent(Artifact actionId) { + return activeActions.get(actionId); + } + void actionStarted(ActionStartedEvent event) { Action action = event.getAction(); Artifact actionId = action.getPrimaryOutput(); @@ -583,10 +588,12 @@ void runningAction(RunningActionEvent event) { } void actionProgress(ActionProgressEvent event) { - ActionExecutionMetadata action = event.action(); Artifact actionId = event.action().getPrimaryOutput(); long now = clock.nanoTime(); - getActionState(action, actionId, now).onProgressEvent(event, now); + ActionState actionState = getActionStateIfPresent(actionId); + if (actionState != null) { + actionState.onProgressEvent(event, now); + } } void actionCompletion(ActionScanningCompletedEvent event) { diff --git a/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java index 37c14736594d77..4fe067a9424182 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java @@ -603,6 +603,7 @@ public void actionProgress_visible() throws Exception { ManualClock clock = new ManualClock(); Action action = createDummyAction("Some random action"); UiStateTracker stateTracker = new UiStateTracker(clock, /* targetWidth= */ 70); + stateTracker.actionStarted(new ActionStartedEvent(action, clock.nanoTime())); stateTracker.actionProgress( ActionProgressEvent.create(action, "action-id", "action progress", false)); LoggingTerminalWriter terminalWriter = new LoggingTerminalWriter(/*discardHighlight=*/ true); @@ -621,6 +622,7 @@ public void actionProgress_withTooSmallWidth_progressSkipped() throws Exception ManualClock clock = new ManualClock(); Action action = createDummyAction("Some random action"); UiStateTracker stateTracker = new UiStateTracker(clock, /* targetWidth= */ 30); + stateTracker.actionStarted(new ActionStartedEvent(action, clock.nanoTime())); stateTracker.actionProgress( ActionProgressEvent.create(action, "action-id", "action progress", false)); LoggingTerminalWriter terminalWriter = new LoggingTerminalWriter(/*discardHighlight=*/ true); @@ -639,6 +641,7 @@ public void actionProgress_withSmallWidth_progressShortened() throws Exception { ManualClock clock = new ManualClock(); Action action = createDummyAction("Some random action"); UiStateTracker stateTracker = new UiStateTracker(clock, /* targetWidth= */ 50); + stateTracker.actionStarted(new ActionStartedEvent(action, clock.nanoTime())); stateTracker.actionProgress( ActionProgressEvent.create(action, "action-id", "action progress", false)); LoggingTerminalWriter terminalWriter = new LoggingTerminalWriter(/*discardHighlight=*/ true); @@ -657,6 +660,7 @@ public void actionProgress_multipleProgress_displayInOrder() throws Exception { ManualClock clock = new ManualClock(); Action action = createDummyAction("Some random action"); UiStateTracker stateTracker = new UiStateTracker(clock, /* targetWidth= */ 70); + stateTracker.actionStarted(new ActionStartedEvent(action, clock.nanoTime())); stateTracker.actionProgress( ActionProgressEvent.create(action, "action-id1", "action progress 1", false)); stateTracker.actionProgress(