diff options
author | Nathan Harmata <nharmata@google.com> | 2015-03-20 18:06:18 +0000 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2015-03-23 11:55:48 +0000 |
commit | fa2995790eb8e27933bf9973f4ae3e16fd8984fd (patch) | |
tree | e1267a1abe4523d2e381417de776a0a9b77284a7 /src/main/java/com/google/devtools/build | |
parent | 5386fa1ef6585948b59daa8e7ab9698a844084c7 (diff) |
Don't bother cleaning the graph if we are crashing anyway, since attempting to do so is very likely to obfuscate the original crash.
--
MOS_MIGRATED_REVID=89138275
Diffstat (limited to 'src/main/java/com/google/devtools/build')
-rw-r--r-- | src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java | 29 |
1 files changed, 18 insertions, 11 deletions
diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java index 55c1ab59c0..b9edeb9c2c 100644 --- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java @@ -1152,12 +1152,25 @@ public final class ParallelEvaluator implements Evaluator { throw new IllegalStateException(entry + " for " + skyKey + " in unknown state"); } } + boolean shouldClean = false; try { - return waitForCompletionAndConstructResult(visitor, skyKeys); + EvaluationResult<T> result = waitForCompletionAndConstructResult(visitor, skyKeys); + shouldClean = true; + return result; + } catch (InterruptedException e) { + shouldClean = true; + throw e; } finally { - // TODO(bazel-team): In nokeep_going mode or in case of an interrupt, we need to remove - // partial values from the graph. Find a better way to handle those cases. - clean(visitor.inflightNodes); + if (shouldClean) { + // TODO(bazel-team): In nokeep_going mode or in case of an interrupt, we need to remove + // partial values from the graph. Find a better way to handle those cases. + clean(visitor.inflightNodes); + } + // If we're going to crash anyway, things might be in a weird state such that 'clean' is + // likely to crash too. So propagating the original exception is much more useful. + // Note that this is safe in the sense that this decision doesn't prevent weird + // SkyFunctions from doing weird things with unchecked exceptions - that's already + // prevented directly by SkyFunctionException#validateExceptionType. } } @@ -1171,13 +1184,7 @@ public final class ParallelEvaluator implements Evaluator { new ThrowableRecordingRunnableWrapper("ParallelEvaluator#clean"); for (final SkyKey key : inflightNodes) { final NodeEntry entry = graph.get(key); - if (entry.isDone()) { - // Entry may be done in case of a RuntimeException or other programming bug. Do nothing, - // since (a) we're about to crash anyway, and (b) getTemporaryDirectDeps cannot be called - // on a done node, so the call below would crash, which would mask the actual exception - // that caused this state. - continue; - } + Preconditions.checkState(!entry.isDone(), key); executor.execute(wrapper.wrap(new Runnable() { @Override public void run() { |