Skip to content

Commit

Permalink
Fix crash when trying to obtain an FilesetOutputTree input in `Acti…
Browse files Browse the repository at this point in the history
…onExecutionFunction`, where the owning analysis node for the Fileset is in the frontier.

PiperOrigin-RevId: 718223413
Change-Id: I7bd1bd0dd3727262e8a144343f4b1e7142e48827
  • Loading branch information
jin authored and fmeum committed Jan 29, 2025
1 parent 5030b24 commit 3fa9b38
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,26 @@ private static FilesetOutputTree getFilesetOutput(Environment env, SpecialArtifa
ActionLookupData generatingActionKey = actionInput.getGeneratingActionKey();
ActionLookupKey filesetActionLookupKey = generatingActionKey.getActionLookupKey();

ActionLookupValue filesetActionLookupValue =
(ActionLookupValue) env.getValue(filesetActionLookupKey);
SkyValue maybeActionLookupValue = env.getValue(filesetActionLookupKey);

// With Frontier-based Skycache, the SkyValue for the fileset analysis object may be a
// RemoteConfiguredTargetValue if it's not in the active set, and the remote fileset CT value
// doesn't contain actions for lookup. That is OK because we're only interested in the fileset
// output, which is not stored in the actions, but in the (potentially Skycache-cached)
// ActionExecutionValue.
//
// Additionally, `if (generatingAction instanceof SymlinkAction)` block below is also only
// triggered when building fileset symlinks, which is disabled in a Skycache build.
//
// So, for a Skycache build, we can skip the transitive action lookups below and obtain the
// FilesetOutputTree directory from the Fileset ActionExecutionValue immediately (which should
// be a Skycache cache hit anyway).
if (maybeActionLookupValue instanceof RemoteConfiguredTargetValue) {
return getFilesetOutputFromKey(generatingActionKey, env);
}

// Non-Skycache builds from here.
ActionLookupValue filesetActionLookupValue = (ActionLookupValue) maybeActionLookupValue;
ActionAnalysisMetadata generatingAction =
filesetActionLookupValue.getAction(generatingActionKey.getActionIndex());
ActionLookupData filesetActionKey;
Expand Down Expand Up @@ -179,13 +196,19 @@ private static FilesetOutputTree getFilesetOutput(Environment env, SpecialArtifa
filesetActionKey = generatingActionKey;
}

// TODO(janakr: properly handle exceptions coming from here, or prove they can never happen in
// practice.
return getFilesetOutputFromKey(filesetActionKey, env);
}

private static FilesetOutputTree getFilesetOutputFromKey(
ActionLookupData filesetActionKey, Environment env) throws InterruptedException {
// TODO: properly handle exceptions coming from here, or prove they can never happen in
// practice.
ActionExecutionValue filesetValue = (ActionExecutionValue) env.getValue(filesetActionKey);
if (filesetValue == null) {
// At this point skyframe does not guarantee that the filesetValue will be ready, since
// the current action does not directly depend on the outputs of the
// SkyframeFilesetManifestAction whose ActionExecutionValue (filesetValue) is needed here.
// SkyframeFilesetManifestAction whose ActionExecutionValue
// (com.google.devtools.build.lib.skyframe.Fileset) is needed here.
return null;
}
return filesetValue.getFilesetOutput();
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ java_library(
deps = [
":action_execution_value",
":metadata_consumer_for_metrics",
":rule_configured_target_value",
":tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data",
Expand Down

0 comments on commit 3fa9b38

Please sign in to comment.