提交 132ca7cf 编写于 作者: O Oleg Nenashev 提交者: GitHub

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

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

This change adds missing NPE checks and also improves handling of Executables#getParentOf(), which may throw undocumented Runtime exceptions if Executable uses core API below 1.377 and does not implement getParent(). Executor#stop() has been also modified to explicitly handle the issue though there is still the same issue with estimated execution times.

* [JENKINS-43653] - Address comments from @jglick, restrict newly introduced API

* [JENKINS-43653] - Address comments from @stephenc

* [JENKINS-43653] - Cleanup "unrelevant" changes to make @stephenc and @jglick happy

* [JENKINS-43653] - Cleanup the leftover

* [JENKINS-43653] - Drop the changes in Executables
上级 cdb45dd2
......@@ -31,6 +31,7 @@ import hudson.Util;
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;
......@@ -87,6 +88,8 @@ import javax.xml.transform.stream.StreamResult;
import javax.xml.transform.stream.StreamSource;
import static hudson.model.queue.Executables.getParentOf;
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,12 @@ public abstract class AbstractItem extends Actionable implements Item, HttpDelet
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;
final SubTask subtask = executable != null ? getParentOf(executable) : null;
if (subtask != null) {
Item item = Tasks.getItemOf(subtask);
if (item != null) {
while (item != null) {
if (item == this) {
......
......@@ -511,6 +511,7 @@ public class Executor extends Thread implements ModelObject {
* null if the executor is idle.
*/
@Exported
@CheckForNull
public WorkUnit getCurrentWorkUnit() {
lock.readLock().lock();
try {
......
......@@ -36,10 +36,14 @@ import javax.annotation.Nonnull;
* @author Kohsuke Kawaguchi
*/
public class Executables {
/**
* Due to the return type change in {@link Executable}, the caller needs a special precaution now.
* Due to the return type change in {@link Executable} in 1.377, the caller needs a special precaution now.
* @param e Executable
* @return Discovered subtask
*/
public static @Nonnull SubTask getParentOf(Executable e) {
public static @Nonnull SubTask getParentOf(@Nonnull Executable e)
throws Error, RuntimeException {
try {
return e.getParent();
} catch (AbstractMethodError _) {
......@@ -77,6 +81,7 @@ public class Executables {
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();
}
}
......
......@@ -79,6 +79,7 @@ public final class WorkUnit {
/**
* If the execution has already started, return the executable that was created.
*/
@CheckForNull
public Executable getExecutable() {
return executable;
}
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册