Skip to content

Commit

Permalink
Clear interrupted bit in thread when throwing an interrupted exceptio…
Browse files Browse the repository at this point in the history
…n that came from an AbstractParallelEvaluator evaluation. It's against the standard Java contract to throw but still have the thread's interrupted bit set.

Also get rid of some unnecessary initializeTester() calls in MemoizingEvaluatorTest: we already call it via a @before annotation.

PiperOrigin-RevId: 176496034
  • Loading branch information
janakdr authored and Copybara-Service committed Nov 21, 2017
1 parent 9bb93ee commit 34d02ef
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -564,8 +564,9 @@ void propagateEvaluatorContextCrashIfAny() {
}

void propagateInterruption(SchedulerException e) throws InterruptedException {
boolean mustThrowInterrupt = Thread.interrupted();
Throwables.propagateIfPossible(e.getCause(), InterruptedException.class);
if (Thread.interrupted()) {
if (mustThrowInterrupt) {
// As per the contract of AbstractQueueVisitor#work, if an unchecked exception is thrown and
// the build is interrupted, the thrown exception is what will be rethrown. Since the user
// presumably wanted to interrupt the build, we ignore the thrown SchedulerException (which
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,19 @@ public void cachedErrorShutsDownThreadpool() throws Exception {
assertThatEvaluationResult(result).hasErrorEntryForKeyThat(newErrorKey).isNull();
}

@Test
public void interruptBitCleared() throws Exception {
SkyKey interruptKey = GraphTester.skyKey("interrupt");
tester.getOrCreate(interruptKey).setBuilder(INTERRUPT_BUILDER);
try {
tester.eval(/*keepGoing=*/ true, interruptKey);
fail("Expected interrupt");
} catch (InterruptedException e) {
// Expected.
}
assertThat(Thread.interrupted()).isFalse();
}

@Test
public void crashAfterInterruptCrashes() throws Exception {
SkyKey failKey = GraphTester.skyKey("fail");
Expand Down Expand Up @@ -1736,7 +1749,6 @@ public void continueWithErrorDep() throws Exception {

@Test
public void continueWithErrorDepTurnedGood() throws Exception {
initializeTester();
SkyKey errorKey = GraphTester.toSkyKey("my_error_value");
tester.getOrCreate(errorKey).setHasError(true);
tester.set("after", new StringValue("after"));
Expand Down Expand Up @@ -2664,7 +2676,6 @@ public void invalidated(SkyKey skyKey, InvalidationState state) {

@Test
public void changePruning() throws Exception {
initializeTester();
SkyKey leaf = GraphTester.toSkyKey("leaf");
SkyKey mid = GraphTester.toSkyKey("mid");
SkyKey top = GraphTester.toSkyKey("top");
Expand Down Expand Up @@ -2722,7 +2733,6 @@ public void changePruningWithDoneValue() throws Exception {

@Test
public void changePruningAfterParentPrunes() throws Exception {
initializeTester();
final SkyKey leaf = GraphTester.toSkyKey("leaf");
SkyKey top = GraphTester.toSkyKey("top");
tester.set(leaf, new StringValue("leafy"));
Expand Down

0 comments on commit 34d02ef

Please sign in to comment.