Properly report completion of shared actions with input discovery

Currently we report "Analyzing" when include scanning runs. But since we can
have shared C++ compile actions, only one of the group will be executed and only
one will be reported completed. Remaining shared actions currently stay with
"analyzing" forever.

This cl makes sure that these actions are properly handled when finished.

This is an encore of 24f19ec. In this incarnation I make sure that all actions that discover inputs are consistent in reporting their Analyzing status. Originally only CppCompileAction was doing that. Apparently we have more actions that discover inputs (e.g. LtoBackendAction) but these were not reporting Analyzing and therefore crashing on preconditions. This cl makes sure that all actions discovering inputs report their analyzing status.

RELNOTES: None
PiperOrigin-RevId: 194066513

src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java

@@ -31,7 +31,6 @@
 import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.ActionResult;
-import com.google.devtools.build.lib.actions.ActionStatusMessage;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactResolver;
 import com.google.devtools.build.lib.actions.CommandAction;
@@ -518,8 +517,6 @@ private Iterable<Artifact> findAdditionalInputs(ActionExecutionContext actionExe
   @Override
   public Iterable<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
-    actionExecutionContext.getEventBus().post(ActionStatusMessage.analysisStrategy(this));
-
     Iterable<Artifact> initialResult = findAdditionalInputs(actionExecutionContext);
 
     if (shouldPruneModules) {

src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java

@@ -191,7 +191,9 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws ActionExecutionFu
 
     ActionExecutionValue result;
     try {
-      result = checkCacheAndExecuteIfNeeded(action, state, env, clientEnv, actionLookupData);
+      result =
+          checkCacheAndExecuteIfNeeded(
+              action, state, env, clientEnv, actionLookupData, sharedActionAlreadyRan);
     } catch (ActionExecutionException e) {
       // Remove action from state map in case it's there (won't be unless it discovers inputs).
       stateMap.remove(action);
@@ -339,13 +341,20 @@ private ActionExecutionValue checkCacheAndExecuteIfNeeded(
       ContinuationState state,
       Environment env,
       Map<String, String> clientEnv,
-      ActionLookupData actionLookupData)
+      ActionLookupData actionLookupData,
+      boolean sharedActionAlreadyRan)
       throws ActionExecutionException, InterruptedException {
     // If this is a shared action and the other action is the one that executed, we must use that
     // other action's value, provided here, since it is populated with metadata for the outputs.
-    if (!state.hasArtifactData()) {
-      return skyframeActionExecutor
-          .executeAction(env.getListener(), action, null, -1, null, actionLookupData);
+    if (sharedActionAlreadyRan) {
+      return skyframeActionExecutor.executeAction(
+          env.getListener(),
+          action,
+          /* metadataHandler= */ null,
+          /* actionStartTime= */ -1,
+          /* actionExecutionContext= */ null,
+          actionLookupData,
+          /* inputDiscoveryRan= */ false);
     }
     // This may be recreated if we discover inputs.
     ActionMetadataHandler metadataHandler = new ActionMetadataHandler(state.inputArtifactData,
@@ -429,8 +438,13 @@ private ActionExecutionValue checkCacheAndExecuteIfNeeded(
       if (!state.hasExecutedAction()) {
         state.value =
             skyframeActionExecutor.executeAction(
-                env.getListener(), action, metadataHandler, actionStartTime, actionExecutionContext,
-                actionLookupData);
+                env.getListener(),
+                action,
+                metadataHandler,
+                actionStartTime,
+                actionExecutionContext,
+                actionLookupData,
+                /* inputDiscoveryRan= */ true);
       }
     } catch (IOException e) {
       throw new ActionExecutionException(

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java

@@ -389,7 +389,8 @@ ActionExecutionValue executeAction(
       ActionMetadataHandler metadataHandler,
       long actionStartTime,
       ActionExecutionContext actionExecutionContext,
-      ActionLookupData actionLookupData)
+      ActionLookupData actionLookupData,
+      boolean inputDiscoveryRan)
       throws ActionExecutionException, InterruptedException {
     Exception exception = badActionMap.get(action);
     if (exception != null) {
@@ -409,8 +410,10 @@ ActionExecutionValue executeAction(
     // Check to see if another action is already executing/has executed this value.
     Pair<ActionLookupData, FutureTask<ActionExecutionValue>> oldAction =
         buildActionMap.putIfAbsent(primaryOutput, Pair.of(actionLookupData, actionTask));
+    // true if this is a non-shared action or it's shared and to be executed.
+    boolean isPrimaryActionForTheValue = oldAction == null;
 
-    if (oldAction == null) {
+    if (isPrimaryActionForTheValue) {
       actionTask.run();
     } else {
       // Wait for other action to finish, so any actions that depend on its outputs can execute.
@@ -423,6 +426,15 @@ ActionExecutionValue executeAction(
           ActionExecutionException.class, InterruptedException.class);
       throw new IllegalStateException(e);
     } finally {
+      if (!isPrimaryActionForTheValue && action.discoversInputs() && inputDiscoveryRan) {
+        /**
+         * If this is a shared action that does input discovery, but was not executed, we need to
+         * remove it from the active actions pool (it was added there by {@link
+         * ActionRunner#call()}).
+         */
+        // TODO(b/72764586): Cleanup once we can properly skip input discovery for shared actions
+        statusReporterRef.get().remove(action);
+      }
       String message = action.getProgressMessage();
       if (message != null) {
         // Tell the receiver that the action has completed *before* telling the reporter.
@@ -602,6 +614,7 @@ Iterable<Artifact> discoverInputs(
             clientEnv,
             env);
     try {
+      actionExecutionContext.getEventBus().post(ActionStatusMessage.analysisStrategy(action));
       return action.discoverInputs(actionExecutionContext);
     } catch (ActionExecutionException e) {
       throw processAndThrow(
@@ -832,7 +845,7 @@ private void prepareScheduleExecuteAndCompleteAction(
           context.getFileOutErr(),
           outputDumped);
     } finally {
-      statusReporter.remove(action);
+      statusReporterRef.get().remove(action);
       eventHandler.post(new ActionCompletionEvent(actionStartTime, action, actionLookupData));
     }
   }

src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java

@@ -65,7 +65,7 @@ public Clock getClock() {
 
   @Override
   public EventBus getEventBus() {
-    throw new UnsupportedOperationException();
+    return new EventBus();
   }
 
   @Override