Skip to content

Commit

Permalink
Check the result of Future.cancel() when cancelling the other branch …
Browse files Browse the repository at this point in the history
…of dynamic execution.

Under --experimental_local_lockfree_output, the local branch may succeed and set the future before cancelling the remote branch. If the remote branch succeeded after that but before the local listener got going, it would falsely think the local was cancelled, which would occasionally lead to strange errors.

RELNOTES: None.
PiperOrigin-RevId: 373347494
  • Loading branch information
larsrc-google authored and Copybara-Service committed May 12, 2021
1 parent ba4435c commit 5b95d91
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
Expand Up @@ -34,6 +34,10 @@ enum DynamicMode {
public String toString() {
return name;
}

public DynamicMode other() {
return this == REMOTE ? LOCAL : REMOTE;
}
}

/**
Expand Down
Expand Up @@ -173,7 +173,17 @@ private static void stopBranch(
spawn.getMnemonic(), strategyThatCancelled.get())));
}

branchToCancel.cancel(true);
if (!branchToCancel.cancel(true)) {
// This can happen if the other branch is local under local_lockfree and has returned
// its result but not yet cancelled this branch, or if the other branch was already
// cancelled for other reasons.
if (!branchToCancel.isCancelled()) {
throw new DynamicInterruptedException(
String.format(
"Execution of %s strategy stopped because %s strategy could not be cancelled",
cancellingStrategy, cancellingStrategy.other()));
}
}
branchDone.acquire();
} else {
throw new DynamicInterruptedException(
Expand Down

0 comments on commit 5b95d91

Please sign in to comment.