diff options
Diffstat (limited to 'src/main')
4 files changed, 110 insertions, 119 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/query2/AbstractBlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/AbstractBlazeQueryEnvironment.java index cd597a66cd..1962cdfaf4 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/AbstractBlazeQueryEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/query2/AbstractBlazeQueryEnvironment.java @@ -46,7 +46,7 @@ import java.util.logging.Logger; * QueryEnvironment as possible while remaining mostly agnostic as to the objects being stored. */ public abstract class AbstractBlazeQueryEnvironment<T> - implements QueryEnvironment<T>, AutoCloseable { + implements QueryEnvironment<T> { protected final ErrorSensingEventHandler eventHandler; protected final boolean keepGoing; protected final boolean strictScope; @@ -146,9 +146,6 @@ public abstract class AbstractBlazeQueryEnvironment<T> return new QueryEvalResult(!eventHandler.hasErrors(), empty.get()); } - @Override - public abstract void close(); - public QueryExpression transformParsedQuery(QueryExpression queryExpression) { return queryExpression; } diff --git a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java index 1b1749fe08..2b8c3e602f 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java @@ -109,13 +109,6 @@ public class BlazeQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> this.labelVisitor = new LabelVisitor(packageProvider, dependencyFilter); } - /** - * Calling close is optional because {@link BlazeQueryEnvironment} has no resources that need - * manual management. - */ - @Override - public void close() {} - @Override public DigraphQueryEvalResult<Target> evaluateQuery(QueryExpression expr, final Callback<Target> callback) throws QueryException, InterruptedException { 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 39385ddd78..51fb19c50a 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 @@ -101,6 +101,7 @@ import java.util.Map.Entry; import java.util.Queue; import java.util.Set; import java.util.concurrent.ForkJoinPool; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -133,11 +134,12 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> private final List<String> universeScope; protected final String parserPrefix; private final PathPackageLocator pkgPath; - private final ForkJoinPool forkJoinPool; + private final int queryEvaluationParallelismLevel; // The following fields are set in the #beforeEvaluateQuery method. protected WalkableGraph graph; private InterruptibleSupplier<ImmutableSet<PathFragment>> blacklistPatternsSupplier; + private ForkJoinPool forkJoinPool; private RecursivePackageProviderBackedTargetPatternResolver resolver; public SkyQueryEnvironment( @@ -188,15 +190,13 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> extraFunctions, evalListener); this.loadingPhaseThreads = loadingPhaseThreads; - // Note that ForkJoinPool doesn't start any thread until work is submitted to it. - this.forkJoinPool = NamedForkJoinPool.newNamedPool( - "QueryEnvironment", queryEvaluationParallelismLevel); this.graphFactory = graphFactory; this.pkgPath = pkgPath; this.universeScope = Preconditions.checkNotNull(universeScope); this.parserPrefix = parserPrefix; - Preconditions.checkState(!universeScope.isEmpty(), - "No queries can be performed with an empty universe"); + Preconditions.checkState( + !universeScope.isEmpty(), "No queries can be performed with an empty universe"); + this.queryEvaluationParallelismLevel = queryEvaluationParallelismLevel; } private void beforeEvaluateQuery() throws InterruptedException { @@ -217,6 +217,8 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> PrepareDepsOfPatternsFunction.getSkyKeys(universeKey, eventHandler)); GraphBackedRecursivePackageProvider graphBackedRecursivePackageProvider = new GraphBackedRecursivePackageProvider(graph, universeTargetPatternKeys, pkgPath); + forkJoinPool = + NamedForkJoinPool.newNamedPool("QueryEnvironment", queryEvaluationParallelismLevel); resolver = new RecursivePackageProviderBackedTargetPatternResolver( graphBackedRecursivePackageProvider, @@ -285,11 +287,6 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> } @Override - public void close() { - forkJoinPool.shutdownNow(); - } - - @Override public final QueryExpression transformParsedQuery(QueryExpression queryExpression) { QueryExpression transformedQueryExpression = getTransformedQueryExpression(queryExpression); LOG.info(String.format( @@ -345,6 +342,12 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> eval(expr, VariableContext.<Target>empty(), callbackWithEmptyCheck); } catch (QueryException e) { throw new QueryException(e, expr); + } finally { + // Force termination of remaining tasks - if evaluateQuery was successful there should be + // none, if it failed abruptly (e.g. was interrupted) we don't want to leave any dangling + // threads running tasks. + forkJoinPool.shutdownNow(); + forkJoinPool.awaitQuiescence(Long.MAX_VALUE, TimeUnit.MILLISECONDS); } aggregator.processLastPending(); } @@ -794,8 +797,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> if (originalFileFragment.equals(currentPathFragment) && originalFileFragment.equals(Label.EXTERNAL_PACKAGE_FILE_NAME)) { Preconditions.checkState( - Label.EXTERNAL_PACKAGE_FILE_NAME.getParentDirectory().equals( - PathFragment.EMPTY_FRAGMENT), + Label.EXTERNAL_PACKAGE_FILE_NAME.getParentDirectory().equals(PathFragment.EMPTY_FRAGMENT), Label.EXTERNAL_PACKAGE_FILE_NAME); return ImmutableList.of( PackageLookupValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER), diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java index 34afe6be5f..3f0438c03a 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java @@ -132,113 +132,112 @@ public final class QueryCommand implements BlazeCommand { Set<Setting> settings = queryOptions.toSettings(); boolean streamResults = QueryOutputUtils.shouldStreamResults(queryOptions, formatter); QueryEvalResult result; - try (AbstractBlazeQueryEnvironment<Target> queryEnv = + AbstractBlazeQueryEnvironment<Target> queryEnv = newQueryEnvironment( - env, - queryOptions.keepGoing, - !streamResults, - queryOptions.universeScope, - queryOptions.loadingPhaseThreads, - settings)) { - // 1. Parse and transform query: - QueryExpression expr; - try { - expr = QueryExpression.parse(query, queryEnv); - } catch (QueryException e) { - env.getReporter() - .handle(Event.error(null, "Error while parsing '" + query + "': " + e.getMessage())); - return ExitCode.COMMAND_LINE_ERROR; - } - expr = queryEnv.transformParsedQuery(expr); + env, + queryOptions.keepGoing, + !streamResults, + queryOptions.universeScope, + queryOptions.loadingPhaseThreads, + settings); + // 1. Parse and transform query: + QueryExpression expr; + try { + expr = QueryExpression.parse(query, queryEnv); + } catch (QueryException e) { + env.getReporter() + .handle(Event.error(null, "Error while parsing '" + query + "': " + e.getMessage())); + return ExitCode.COMMAND_LINE_ERROR; + } + expr = queryEnv.transformParsedQuery(expr); - PrintStream output = null; - OutputFormatterCallback<Target> callback; - if (streamResults) { - disableAnsiCharactersFiltering(env); - output = new PrintStream(env.getReporter().getOutErr().getOutputStream()); - // 2. Evaluate expression: - StreamedFormatter streamedFormatter = ((StreamedFormatter) formatter); - streamedFormatter.setOptions( - queryOptions, - queryOptions.aspectDeps.createResolver(env.getPackageManager(), env.getReporter())); - callback = streamedFormatter.createStreamCallback(output, queryOptions, queryEnv); + PrintStream output = null; + OutputFormatterCallback<Target> callback; + if (streamResults) { + disableAnsiCharactersFiltering(env); + output = new PrintStream(env.getReporter().getOutErr().getOutputStream()); + // 2. Evaluate expression: + StreamedFormatter streamedFormatter = ((StreamedFormatter) formatter); + streamedFormatter.setOptions( + queryOptions, + queryOptions.aspectDeps.createResolver(env.getPackageManager(), env.getReporter())); + callback = streamedFormatter.createStreamCallback(output, queryOptions, queryEnv); + } else { + callback = new AggregateAllOutputFormatterCallback<>(); + } + boolean catastrophe = true; + try { + callback.start(); + result = queryEnv.evaluateQuery(expr, callback); + catastrophe = false; + } catch (QueryException e) { + catastrophe = false; + // Keep consistent with reportBuildFileError() + env.getReporter() + // TODO(bazel-team): this is a kludge to fix a bug observed in the wild. We should make + // sure no null error messages ever get in. + .handle(Event.error(e.getMessage() == null ? e.toString() : e.getMessage())); + return ExitCode.ANALYSIS_FAILURE; + } catch (InterruptedException e) { + catastrophe = false; + IOException ioException = callback.getIoException(); + if (ioException == null || ioException instanceof ClosedByInterruptException) { + env.getReporter().handle(Event.error("query interrupted")); + return ExitCode.INTERRUPTED; } else { - callback = new AggregateAllOutputFormatterCallback<>(); + env.getReporter().handle(Event.error("I/O error: " + e.getMessage())); + return ExitCode.LOCAL_ENVIRONMENTAL_ERROR; } - boolean catastrophe = true; - try { - callback.start(); - result = queryEnv.evaluateQuery(expr, callback); - catastrophe = false; - } catch (QueryException e) { - catastrophe = false; - // Keep consistent with reportBuildFileError() - env.getReporter() - // TODO(bazel-team): this is a kludge to fix a bug observed in the wild. We should make - // sure no null error messages ever get in. - .handle(Event.error(e.getMessage() == null ? e.toString() : e.getMessage())); - return ExitCode.ANALYSIS_FAILURE; - } catch (InterruptedException e) { - catastrophe = false; - IOException ioException = callback.getIoException(); - if (ioException == null || ioException instanceof ClosedByInterruptException) { - env.getReporter().handle(Event.error("query interrupted")); - return ExitCode.INTERRUPTED; - } else { + } catch (IOException e) { + catastrophe = false; + env.getReporter().handle(Event.error("I/O error: " + e.getMessage())); + return ExitCode.LOCAL_ENVIRONMENTAL_ERROR; + } finally { + if (!catastrophe) { + if (streamResults) { + output.flush(); + } + try { + callback.close(); + } catch (IOException e) { env.getReporter().handle(Event.error("I/O error: " + e.getMessage())); return ExitCode.LOCAL_ENVIRONMENTAL_ERROR; } + } + } + + env.getEventBus().post(new NoBuildEvent()); + if (!streamResults) { + disableAnsiCharactersFiltering(env); + output = new PrintStream(env.getReporter().getOutErr().getOutputStream()); + + // 3. Output results: + try { + Set<Target> targets = + ((AggregateAllOutputFormatterCallback<Target>) callback).getOutput(); + QueryOutputUtils.output( + queryOptions, + result, + targets, + formatter, + output, + queryOptions.aspectDeps.createResolver(env.getPackageManager(), env.getReporter())); + } catch (ClosedByInterruptException | InterruptedException e) { + env.getReporter().handle(Event.error("query interrupted")); + return ExitCode.INTERRUPTED; } catch (IOException e) { - catastrophe = false; env.getReporter().handle(Event.error("I/O error: " + e.getMessage())); return ExitCode.LOCAL_ENVIRONMENTAL_ERROR; } finally { - if (!catastrophe) { - if (streamResults) { - output.flush(); - } - try { - callback.close(); - } catch (IOException e) { - env.getReporter().handle(Event.error("I/O error: " + e.getMessage())); - return ExitCode.LOCAL_ENVIRONMENTAL_ERROR; - } - } - } - - env.getEventBus().post(new NoBuildEvent()); - if (!streamResults) { - disableAnsiCharactersFiltering(env); - output = new PrintStream(env.getReporter().getOutErr().getOutputStream()); - - // 3. Output results: - try { - Set<Target> targets = - ((AggregateAllOutputFormatterCallback<Target>) callback).getOutput(); - QueryOutputUtils.output( - queryOptions, - result, - targets, - formatter, - output, - queryOptions.aspectDeps.createResolver(env.getPackageManager(), env.getReporter())); - } catch (ClosedByInterruptException | InterruptedException e) { - env.getReporter().handle(Event.error("query interrupted")); - return ExitCode.INTERRUPTED; - } catch (IOException e) { - env.getReporter().handle(Event.error("I/O error: " + e.getMessage())); + // Note that PrintStream#checkError first flushes and then returns whether any + // error was ever encountered. + if (output.checkError()) { + // Unfortunately, there's no way to check the current error status of PrintStream + // without forcing a flush, so we don't know whether this error happened before or after + // timewise the one we already have from above. Neither choice is always correct, so we + // arbitrarily choose the exit code corresponding to the PrintStream's error. + env.getReporter().handle(Event.error("I/O error while writing query output")); return ExitCode.LOCAL_ENVIRONMENTAL_ERROR; - } finally { - // Note that PrintStream#checkError first flushes and then returns whether any - // error was ever encountered. - if (output.checkError()) { - // Unfortunately, there's no way to check the current error status of PrintStream - // without forcing a flush, so we don't know whether this error happened before or after - // timewise the one we already have from above. Neither choice is always correct, so we - // arbitrarily choose the exit code corresponding to the PrintStream's error. - env.getReporter().handle(Event.error("I/O error while writing query output")); - return ExitCode.LOCAL_ENVIRONMENTAL_ERROR; - } } } } |