[JENKINS-28690] Aha! So I believe this will fully resolve any of thes…

…e kinds of deadlocks - Without this, then it becomes a question of find catch and release for each potential code path that might end up restoring the interrupt flag on the current thread. - Since standard Lock support is kind enough to restore the interrupt flag on the current thread when blocked waiting for the lock, that would be a hiding to nothing - I welcome others to review my logic detailed in the code comment - I am leaving the code comment as this is IMHO too important to assume that somebody will check the git commit history
jenkinsci · Aug 6, 2015 · 0ba505b · 0ba505b
1 parent d331c12
commit 0ba505b
Showing 1 changed file with 28 additions and 1 deletion.
diff --git a/core/src/main/java/hudson/model/Executor.java b/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() {