[JENKINS-43653] - Ensure AbstractItem#delete() NPE safety when checking executors

core/src/main/java/hudson/model/AbstractItem.java

@@ -31,6 +31,7 @@
 import hudson.Functions;
 import hudson.BulkChange;
 import hudson.cli.declarative.CLIResolver;
+import hudson.model.Queue.Executable;
 import hudson.model.listeners.ItemListener;
 import hudson.model.listeners.SaveableListener;
 import hudson.model.queue.Tasks;
@@ -86,7 +87,9 @@
 import javax.xml.transform.stream.StreamResult;
 import javax.xml.transform.stream.StreamSource;
 
-import static hudson.model.queue.Executables.getParentOf;
+import static hudson.model.queue.Executables.getParentOfOrFail;
+import hudson.model.queue.SubTask;
+import java.lang.reflect.InvocationTargetException;
 import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
 import org.apache.commons.io.FileUtils;
 import org.kohsuke.accmod.Restricted;
@@ -620,9 +623,20 @@ public void delete() throws IOException, InterruptedException {
                 Map<Executor, Queue.Executable> buildsInProgress = new LinkedHashMap<>();
                 for (Computer c : Jenkins.getInstance().getComputers()) {
                     for (Executor e : c.getAllExecutors()) {
-                        WorkUnit workUnit = e.getCurrentWorkUnit();
-                        if (workUnit != null) {
-                            Item item = Tasks.getItemOf(getParentOf(workUnit.getExecutable()));
+                        final WorkUnit workUnit = e.getCurrentWorkUnit();
+                        final Executable executable = workUnit != null ? workUnit.getExecutable() : null;
+                        SubTask subtask = null;
+                        if (executable != null) {
+                            try {
+                                subtask = getParentOfOrFail(executable);
+                            } catch(InvocationTargetException ex) {
+                                // Executable is not compatible with API changes in 1.377+
+                                LOGGER.log(Level.WARNING, "Cannot determine subtask for the executable with obsolete API implementation", ex);
+                            }
+                        }
+
+                        if (subtask != null) {        
+                            Item item = Tasks.getItemOf(subtask);
                             if (item != null) {
                                 while (item != null) {
                                     if (item == this) {

core/src/main/java/hudson/model/Executor.java

@@ -64,6 +64,7 @@
 import java.util.logging.Logger;
 
 import static hudson.model.queue.Executables.*;
+import java.lang.reflect.InvocationTargetException;
 import java.util.Collection;
 import static java.util.logging.Level.*;
 import javax.annotation.CheckForNull;
@@ -511,6 +512,7 @@ public void completedAsynchronous(@CheckForNull Throwable error) {
      *      null if the executor is idle.
      */
     @Exported
+    @CheckForNull
     public WorkUnit getCurrentWorkUnit() {
         lock.readLock().lock();
         try {
@@ -843,7 +845,13 @@ public HttpResponse doStop() {
         lock.writeLock().lock(); // need write lock as interrupt will change the field
         try {
             if (executable != null) {
-                Tasks.getOwnerTaskOf(getParentOf(executable)).checkAbortPermission();
+                final SubTask parentOf;
+                try {
+                   parentOf = getParentOfOrFail(executable);
+                } catch(InvocationTargetException ex) {
+                    return HttpResponses.error(500, ex);
+                }
+                Tasks.getOwnerTaskOf(parentOf).checkAbortPermission();
                 interrupt();
             }
         } finally {

core/src/main/java/hudson/model/queue/Executables.java

@@ -29,17 +29,29 @@
 import java.lang.reflect.Method;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * Convenience methods around {@link Executable}.
  *
  * @author Kohsuke Kawaguchi
  */
 public class Executables {
+
+    // TODO: Deprecate getParentOf() and make the new API public
+    // @deprecated This method may throw Runtime exceptions for old cores
+    // Use {@link #getParentOfOrFail(hudson.model.Queue.Executable)} or {@link #getParentOfOrNull(hudson.model.Queue.Executable)} instead.
+
     /**
      * Due to the return type change in {@link Executable}, the caller needs a special precaution now.
+     * @param e Executable
+     * @return Discovered subtask
+     * @throws Error Executable type does not offer the {@link Executable#getParent()} method or it fails with {@link Error}
+     * @throws RuntimeException {@link Executable#getParent()} method fails with {@link Error}
      */
-    public static @Nonnull SubTask getParentOf(Executable e) {
+    public static @Nonnull SubTask getParentOf(@Nonnull Executable e) 
+            throws Error, RuntimeException {
         try {
             return e.getParent();
         } catch (AbstractMethodError _) {
@@ -59,6 +71,29 @@ public class Executables {
             }
         }
     }
+
+    /**
+     * Get parent subtask from which the executable has been created.
+     * @param e Executable
+     * @return Parent subtask from which the executable has been created
+     * @throws InvocationTargetException Operation failure due to the usage of incompatible API for old plugin depending on a core older than 1.377
+     * @since TODO
+     */
+    @Nonnull
+    @Restricted(NoExternalUse.class)
+    public static SubTask getParentOfOrFail(@Nonnull Executable e) throws InvocationTargetException {
+       try {
+            return getParentOf(e);
+        } catch(RuntimeException | Error ex) {
+            throw new InvocationTargetException(ex, formatUnsupportedExecutableAPIMessage(e));
+        } 
+    }
+
+    @Nonnull
+    private static String formatUnsupportedExecutableAPIMessage(@Nonnull Executable e) {
+        return String.format("Cannot retrieve parent subtask of executable %s implementing API version below 1.377 (%s)", 
+                    new Object[] {e, e.getClass()});
+    }
 
     /**
      * Returns the estimated duration for the executable.
@@ -77,6 +112,7 @@ public static long getEstimatedDurationFor(@CheckForNull Executable e) {
         try {
             return e.getEstimatedDuration();
         } catch (AbstractMethodError error) {
+            // TODO: according to the code above, e.getparent() may fail. The method also needs to be reworked
             return e.getParent().getEstimatedDuration();
         }
     }

core/src/main/java/hudson/model/queue/WorkUnit.java

@@ -79,6 +79,7 @@ public void setExecutor(@CheckForNull Executor e) {
     /**
      * If the execution has already started, return the executable that was created.
      */
+    @CheckForNull
     public Executable getExecutable() {
         return executable;
     }