Fix threadpool leak in SkyQueryEnvironment
Shutdown the SkyQueryEnvironment's threadpool after query evaluation
is complete and the environment is ready for disposal.
--
MOS_MIGRATED_REVID=125975317
diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
index 3a8f9ba..5f8749d 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
@@ -38,6 +38,7 @@
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.collect.CompactHashSet;
+import com.google.devtools.build.lib.concurrent.ExecutorUtil;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
@@ -119,9 +120,14 @@
// TODO(janakr): Unify with RecursivePackageProviderBackedTargetPatternResolver's constant.
private static final int BATCH_CALLBACK_SIZE = 10000;
private static final int DEFAULT_THREAD_COUNT = Runtime.getRuntime().availableProcessors();
-
- protected WalkableGraph graph;
- private Supplier<ImmutableSet<PathFragment>> blacklistPatternsSupplier;
+ private static final Logger LOG = Logger.getLogger(SkyQueryEnvironment.class.getName());
+ private static final Function<Target, Label> TARGET_LABEL_FUNCTION =
+ new Function<Target, Label>() {
+ @Override
+ public Label apply(Target target) {
+ return target.getLabel();
+ }
+ };
private final BlazeTargetAccessor accessor = new BlazeTargetAccessor(this);
private final int loadingPhaseThreads;
@@ -130,39 +136,19 @@
private final String parserPrefix;
private final PathPackageLocator pkgPath;
- private static final Logger LOG = Logger.getLogger(SkyQueryEnvironment.class.getName());
-
- private static final Function<Target, Label> TARGET_LABEL_FUNCTION =
- new Function<Target, Label>() {
-
- @Override
- public Label apply(Target target) {
- return target.getLabel();
- }
- };
-
+ // Note that the executor returned by Executors.newFixedThreadPool doesn't start any threads
+ // unless work is submitted to it.
private final ListeningExecutorService threadPool =
MoreExecutors.listeningDecorator(
Executors.newFixedThreadPool(
DEFAULT_THREAD_COUNT,
- new ThreadFactoryBuilder().setNameFormat("GetPackages-%d").build()));
+ new ThreadFactoryBuilder().setNameFormat("QueryEnvironment-%d").build()));
+
+ // The following fields are set in the #beforeEvaluateQuery method.
+ protected WalkableGraph graph;
+ private Supplier<ImmutableSet<PathFragment>> blacklistPatternsSupplier;
private RecursivePackageProviderBackedTargetPatternResolver resolver;
- private static class BlacklistSupplier implements Supplier<ImmutableSet<PathFragment>> {
- private final WalkableGraph graph;
-
- BlacklistSupplier(WalkableGraph graph) {
- this.graph = graph;
- }
-
- @Override
- public ImmutableSet<PathFragment> get() {
- return ((BlacklistedPackagePrefixesValue)
- graph.getValue(BlacklistedPackagePrefixesValue.key()))
- .getPatterns();
- }
- }
-
public SkyQueryEnvironment(
boolean keepGoing,
int loadingPhaseThreads,
@@ -189,17 +175,19 @@
"No queries can be performed with an empty universe");
}
- private void init() throws InterruptedException {
+ private void beforeEvaluateQuery() throws InterruptedException {
EvaluationResult<SkyValue> result;
try (AutoProfiler p = AutoProfiler.logged("evaluation and walkable graph", LOG)) {
- result = graphFactory.prepareAndGet(universeScope, parserPrefix, loadingPhaseThreads,
- eventHandler);
+ result =
+ graphFactory.prepareAndGet(
+ universeScope, parserPrefix, loadingPhaseThreads, eventHandler);
}
- graph = result.getWalkableGraph();
+ SkyKey universeKey = graphFactory.getUniverseKey(universeScope, parserPrefix);
+ checkEvaluationResult(result, universeKey);
+ graph = result.getWalkableGraph();
blacklistPatternsSupplier = Suppliers.memoize(new BlacklistSupplier(graph));
- SkyKey universeKey = graphFactory.getUniverseKey(universeScope, parserPrefix);
ImmutableList<TargetPatternKey> universeTargetPatternKeys =
PrepareDepsOfPatternsFunction.getTargetPatternKeys(
PrepareDepsOfPatternsFunction.getSkyKeys(universeKey, eventHandler));
@@ -211,23 +199,38 @@
eventHandler,
TargetPatternEvaluator.DEFAULT_FILTERING_POLICY,
threadPool);
+ }
- // The prepareAndGet call above evaluates a single PrepareDepsOfPatterns SkyKey.
- // We expect to see either a single successfully evaluated value or a cycle in the result.
+ /**
+ * The {@link EvaluationResult} is from the evaluation of a single PrepareDepsOfPatterns node. We
+ * expect to see either a single successfully evaluated value or a cycle in the result.
+ */
+ private void checkEvaluationResult(EvaluationResult<SkyValue> result, SkyKey universeKey) {
Collection<SkyValue> values = result.values();
if (!values.isEmpty()) {
- Preconditions.checkState(values.size() == 1, "Universe query \"%s\" returned multiple"
- + " values unexpectedly (%s values in result)", universeScope, values.size());
+ Preconditions.checkState(
+ values.size() == 1,
+ "Universe query \"%s\" returned multiple values unexpectedly (%s values in result)",
+ universeScope,
+ values.size());
Preconditions.checkNotNull(result.get(universeKey), result);
} else {
// No values in the result, so there must be an error. We expect the error to be a cycle.
boolean foundCycle = !Iterables.isEmpty(result.getError().getCycleInfo());
- Preconditions.checkState(foundCycle, "Universe query \"%s\" failed with non-cycle error: %s",
- universeScope, result.getError());
+ Preconditions.checkState(
+ foundCycle,
+ "Universe query \"%s\" failed with non-cycle error: %s",
+ universeScope,
+ result.getError());
}
}
@Override
+ public void close() {
+ ExecutorUtil.interruptibleShutdown(threadPool);
+ }
+
+ @Override
public QueryExpression transformParsedQuery(QueryExpression queryExpression) {
// Transform each occurrence of an expressions of the form 'rdeps(<universeScope>, <T>)' to
// 'allrdeps(<T>)'. The latter is more efficient.
@@ -270,7 +273,7 @@
// result is set to have an error iff there were errors emitted during the query, so we reset
// errors here.
eventHandler.resetErrors();
- init();
+ beforeEvaluateQuery();
// SkyQueryEnvironment batches callback invocations using a BatchStreamedCallback, created here
// so that there's one per top-level evaluateQuery call. The batch size is large enough that
@@ -651,8 +654,8 @@
protected void preloadOrThrow(QueryExpression caller, Collection<String> patterns)
throws QueryException, TargetParsingException {
// SkyQueryEnvironment directly evaluates target patterns in #getTarget and similar methods
- // using its graph, which is prepopulated using the universeScope (see #init), so no
- // preloading of target patterns is necessary.
+ // using its graph, which is prepopulated using the universeScope (see #beforeEvaluateQuery),
+ // so no preloading of target patterns is necessary.
}
private static final Function<SkyKey, Label> SKYKEY_TO_LABEL = new Function<SkyKey, Label>() {
@@ -872,6 +875,21 @@
.build();
}
+ private static class BlacklistSupplier implements Supplier<ImmutableSet<PathFragment>> {
+ private final WalkableGraph graph;
+
+ BlacklistSupplier(WalkableGraph graph) {
+ this.graph = graph;
+ }
+
+ @Override
+ public ImmutableSet<PathFragment> get() {
+ return ((BlacklistedPackagePrefixesValue)
+ graph.getValue(BlacklistedPackagePrefixesValue.key()))
+ .getPatterns();
+ }
+ }
+
@ThreadSafe
private static class ConcurrentUniquifier implements Uniquifier<Target> {
