Skip to content
Permalink
Browse files

Merge pull request #1786 from stephenc/jenkins-28690-correct-interrup…

…t-override

[JENKINS-28690] Aha! So I believe this will fully resolve any of these kinds of deadlocks
  • Loading branch information...
stephenc committed Aug 10, 2015
2 parents 7ab8168 + 0ba505b commit a80972307e03a7f67e97dee700720cb80f7f65d8
Showing with 28 additions and 1 deletion.
  1. +28 −1 core/src/main/java/hudson/model/Executor.java
@@ -142,7 +142,34 @@ public Executor(@Nonnull Computer owner, int n) {

@Override
public void interrupt() {
interrupt(Result.ABORTED);
if (Thread.currentThread() == this) {
// If you catch an InterruptedException the "correct" options are limited to one of two choices:
// 1. Propagate the exception;
// 2. Restore the Thread.currentThread().interrupted() flag
// The JVM locking support assumes such behaviour.
// Evil Jenkins overrides the interrupt() method so that when a different thread interrupts this thread
// we abort the build.
// but that causes JENKINS-28690 style deadlocks when the correctly written code does
//
// try {
// ... some long running thing ...
// } catch (InterruptedException e) {
// ... some tidy up
// // restore interrupted flag
// Thread.currentThread().interrupted();
// }
//
// What about why we do not set the Result.ABORTED on this branch?
// That is a good question to ask, the answer is that the only time a thread should be restoring
// its own interrupted flag is when that thread has already been interrupted by another thread
// as such we should assume that the result has already been applied. If that assumption were
// incorrect, then the Run.execute's catch (InterruptedException) block will either set the result
// or have been escaped - in which case the result of the run has been sealed anyway so it does not
// matter.
super.interrupt();
} else {
interrupt(Result.ABORTED);
}
}

void interruptForShutdown() {

0 comments on commit a809723

Please sign in to comment.
You can’t perform that action at this time.