Skip to content

Commit

Permalink
Fix rare crash in dynamic execution where both branches got cancelled.
Browse files Browse the repository at this point in the history
Races in `stopBranch` caused the losing branch to throw `DynamicInterruptedException`, which could then get to that branch's listener before the winning branch actually cancelled.

RELNOTES: None.
PiperOrigin-RevId: 383126912
  • Loading branch information
larsrc-google authored and copybara-github committed Jul 5, 2021
1 parent 4726cbb commit fe1e25b
Showing 1 changed file with 20 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -571,13 +571,21 @@ ImmutableList<SpawnResult> callImpl(ActionExecutionContext context)
DynamicSpawnStrategy.this.options,
actionExecutionContext,
spawn));
} catch (DynamicInterruptedException e) {
// This exception can be thrown due to races in stopBranch(), in which case
// the branch that lost the race may not have been cancelled yet. Cancel it here
// to prevent the listener from cross-cancelling.
localBranch.cancel(true);
throw e;
} catch (
@SuppressWarnings("InterruptedExceptionSwallowed")
Throwable e) {
if (options.debugSpawnScheduler) {
logger.atInfo().log(
"Local branch of %s failed with %s",
spawn.getResourceOwner().prettyPrint(), e.getMessage());
"Local branch of %s failed with %s: '%s'",
spawn.getResourceOwner().prettyPrint(),
e.getClass().getSimpleName(),
e.getMessage());
}
throw e;
} finally {
Expand Down Expand Up @@ -627,13 +635,21 @@ public ImmutableList<SpawnResult> callImpl(ActionExecutionContext context)
spawn));
delayLocalExecution.set(true);
return spawnResults;
} catch (DynamicInterruptedException e) {
// This exception can be thrown due to races in stopBranch(), in which case
// the branch that lost the race may not have been cancelled yet. Cancel it here
// to prevent the listener from cross-cancelling.
remoteBranch.cancel(true);
throw e;
} catch (
@SuppressWarnings("InterruptedExceptionSwallowed")
Throwable e) {
if (options.debugSpawnScheduler) {
logger.atInfo().log(
"Remote branch of %s failed with %s",
spawn.getResourceOwner().prettyPrint(), e.getMessage());
"Remote branch of %s failed with %s: '%s'",
spawn.getResourceOwner().prettyPrint(),
e.getClass().getSimpleName(),
e.getMessage());
}
throw e;
} finally {
Expand Down

0 comments on commit fe1e25b

Please sign in to comment.