Skip to content

Commit

Permalink
Fix an issue with cached EventHandlers in SkyQueryEnvironment's resolver
Browse files Browse the repository at this point in the history
Because the event handler's inner handlers are removed after each query
command, caching the handler caused a subset of subsequent commands'
errors (those reported through the resolver's handler) to go unlogged.

Also fix a bug with cycle detection in DelegatingWalkableGraph.

--
PiperOrigin-RevId: 145124271
MOS_MIGRATED_REVID=145124271
  • Loading branch information
anakanemison authored and laszlocsomor committed Jan 23, 2017
1 parent ec857c7 commit 09f42d5
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ protected SkyQueryEnvironment(
}

private void beforeEvaluateQuery() throws InterruptedException {
boolean resolverNeedsRecreation = false;
if (graph == null || !graphFactory.isUpToDate(universeKey)) {
// If this environment is uninitialized or the graph factory needs to evaluate, do so. We
// assume here that this environment cannot be initialized-but-stale if the factory is up
Expand All @@ -224,21 +223,17 @@ private void beforeEvaluateQuery() throws InterruptedException {

graphBackedRecursivePackageProvider =
new GraphBackedRecursivePackageProvider(graph, universeTargetPatternKeys, pkgPath);
resolverNeedsRecreation = true;
}
if (forkJoinPool == null) {
forkJoinPool =
NamedForkJoinPool.newNamedPool("QueryEnvironment", queryEvaluationParallelismLevel);
resolverNeedsRecreation = true;
}
if (resolverNeedsRecreation) {
resolver =
new RecursivePackageProviderBackedTargetPatternResolver(
graphBackedRecursivePackageProvider,
eventHandler,
TargetPatternEvaluator.DEFAULT_FILTERING_POLICY,
packageSemaphore);
}
resolver =
new RecursivePackageProviderBackedTargetPatternResolver(
graphBackedRecursivePackageProvider,
eventHandler,
TargetPatternEvaluator.DEFAULT_FILTERING_POLICY,
packageSemaphore);
}

protected MultisetSemaphore<PackageIdentifier> makeFreshPackageMultisetSemaphore() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package com.google.devtools.build.skyframe;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.skyframe.QueryableGraph.Reason;
Expand Down Expand Up @@ -92,7 +93,7 @@ public boolean isCycle(SkyKey key) throws InterruptedException {
return false;
}
ErrorInfo errorInfo = entry.getErrorInfo();
return errorInfo != null && errorInfo.getCycleInfo() != null;
return errorInfo != null && !Iterables.isEmpty(errorInfo.getCycleInfo());
}

@Nullable
Expand Down

0 comments on commit 09f42d5

Please sign in to comment.