Skip to content

Commit

Permalink
ItemListener.onCheckDelete & ItemDeletion.cancelBuildsInProgress (j…
Browse files Browse the repository at this point in the history 
…enkinsci#9480)

* `ItemListener.onCheckDelete` & `ItemDeletion.cancelBuildsInProgress`

* Remove missing parameter from javadoc

---------

Co-authored-by: Vincent Latombe <vincent@latombe.net>
  • Loading branch information
@jglick @Vlatombe
jglick and Vlatombe authored Jul 24, 2024
1 parent 0d5232f commit 220165f
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 91 deletions.
94 changes: 4 additions & 90 deletions core/src/main/java/hudson/model/AbstractItem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

package hudson.model;

import static hudson.model.queue.Executables.getParentOf;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;

import com.infradna.tool.bridge_method_injector.WithBridgeMethods;
Expand All @@ -39,9 +38,6 @@
import hudson.cli.declarative.CLIResolver;
import hudson.model.listeners.ItemListener;
import hudson.model.listeners.SaveableListener;
import hudson.model.queue.SubTask;
import hudson.model.queue.Tasks;
import hudson.model.queue.WorkUnit;
import hudson.security.ACL;
import hudson.security.ACLContext;
import hudson.security.AccessControlled;
Expand All @@ -57,12 +53,8 @@
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -705,11 +697,13 @@ public void delete(StaplerRequest req, StaplerResponse rsp) throws IOException,
*
* <p>
* Any exception indicates the deletion has failed, but {@link AbortException} would prevent the caller
* from showing the stack trace. This
* from showing the stack trace.
* @see ItemDeletion
*/
@Override
public void delete() throws IOException, InterruptedException {
checkPermission(DELETE);
ItemListener.checkBeforeDelete(this);
boolean responsibleForAbortingBuilds = !ItemDeletion.contains(this);
boolean ownsRegistration = ItemDeletion.register(this);
if (!ownsRegistration && ItemDeletion.isRegistered(this)) {
Expand All @@ -719,87 +713,7 @@ public void delete() throws IOException, InterruptedException {
try {
// if a build is in progress. Cancel it.
if (responsibleForAbortingBuilds || ownsRegistration) {
Queue queue = Queue.getInstance();
if (this instanceof Queue.Task) {
// clear any items in the queue so they do not get picked up
queue.cancel((Queue.Task) this);
}
// now cancel any child items - this happens after ItemDeletion registration, so we can use a snapshot
for (Queue.Item i : queue.getItems()) {
Item item = Tasks.getItemOf(i.task);
while (item != null) {
if (item == this) {
if (!queue.cancel(i)) {
LOGGER.warning(() -> "failed to cancel " + i);
}
break;
}
if (item.getParent() instanceof Item) {
item = (Item) item.getParent();
} else {
break;
}
}
}
// interrupt any builds in progress (and this should be a recursive test so that folders do not pay
// the 15 second delay for every child item). This happens after queue cancellation, so will be
// a complete set of builds in flight
Map<Executor, Queue.Executable> buildsInProgress = new LinkedHashMap<>();
for (Computer c : Jenkins.get().getComputers()) {
for (Executor e : c.getAllExecutors()) {
final WorkUnit workUnit = e.getCurrentWorkUnit();
final Queue.Executable executable = workUnit != null ? workUnit.getExecutable() : null;
final SubTask subtask = executable != null ? getParentOf(executable) : null;

if (subtask != null) {
Item item = Tasks.getItemOf(subtask);
while (item != null) {
if (item == this) {
buildsInProgress.put(e, e.getCurrentExecutable());
e.interrupt(Result.ABORTED);
break;
}
if (item.getParent() instanceof Item) {
item = (Item) item.getParent();
} else {
break;
}
}
}
}
}
if (!buildsInProgress.isEmpty()) {
// give them 15 seconds or so to respond to the interrupt
long expiration = System.nanoTime() + TimeUnit.SECONDS.toNanos(15);
// comparison with executor.getCurrentExecutable() == computation currently should always be true
// as we no longer recycle Executors, but safer to future-proof in case we ever revisit recycling
while (!buildsInProgress.isEmpty() && expiration - System.nanoTime() > 0L) {
// we know that ItemDeletion will prevent any new builds in the queue
// ItemDeletion happens-before Queue.cancel so we know that the Queue will stay clear
// Queue.cancel happens-before collecting the buildsInProgress list
// thus buildsInProgress contains the complete set we need to interrupt and wait for
for (Iterator<Map.Entry<Executor, Queue.Executable>> iterator =
buildsInProgress.entrySet().iterator();
iterator.hasNext(); ) {
Map.Entry<Executor, Queue.Executable> entry = iterator.next();
// comparison with executor.getCurrentExecutable() == executable currently should always be
// true as we no longer recycle Executors, but safer to future-proof in case we ever
// revisit recycling.
if (!entry.getKey().isAlive()
|| entry.getValue() != entry.getKey().getCurrentExecutable()) {
iterator.remove();
}
// I don't know why, but we have to keep interrupting
entry.getKey().interrupt(Result.ABORTED);
}
Thread.sleep(50L);
}
if (!buildsInProgress.isEmpty()) {
throw new Failure(Messages.AbstractItem_FailureToStopBuilds(
buildsInProgress.size(), getFullDisplayName()
));
}
}
ItemDeletion.cancelBuildsInProgress(this);
}
if (this instanceof ItemGroup) {
// delete individual items first
Expand Down
25 changes: 25 additions & 0 deletions core/src/main/java/hudson/model/listeners/ItemListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.util.Listeners;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

/**
* Receives notifications about CRUD operations of {@link Item}.
Expand Down Expand Up @@ -94,6 +96,16 @@ public void onCopied(Item src, Item item) {
public void onLoaded() {
}

/**
* Called before an item is deleted, providing the ability to veto the deletion operation before it starts.
* @param item the item being deleted
* @throws Failure to veto the operation.
* @throws InterruptedException If a blocking condition was interrupted, also vetoing the operation.
* @since TODO
*/
public void onCheckDelete(Item item) throws Failure, InterruptedException {
}

/**
* Called right before a job is going to be deleted.
*
Expand Down Expand Up @@ -205,6 +217,19 @@ public static void fireOnUpdated(final Item item) {
Listeners.notify(ItemListener.class, false, l -> l.onUpdated(item));
}

@Restricted(NoExternalUse.class)
public static void checkBeforeDelete(Item item) throws Failure, InterruptedException {
for (ItemListener l : all()) {
try {
l.onCheckDelete(item);
} catch (Failure e) {
throw e;
} catch (RuntimeException x) {
LOGGER.log(Level.WARNING, "failed to send event to listener of " + l.getClass(), x);
}
}
}

/** @since 1.548 */
public static void fireOnDeleted(final Item item) {
Listeners.notify(ItemListener.class, false, l -> l.onDeleted(item));
Expand Down
109 changes: 108 additions & 1 deletion core/src/main/java/jenkins/model/queue/ItemDeletion.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,42 @@
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.ExtensionList;
import hudson.model.AbstractItem;
import hudson.model.Action;
import hudson.model.Computer;
import hudson.model.Executor;
import hudson.model.Failure;
import hudson.model.Item;
import hudson.model.Messages;
import hudson.model.Queue;
import hudson.model.Result;
import hudson.model.queue.Executables;
import hudson.model.queue.SubTask;
import hudson.model.queue.Tasks;
import hudson.model.queue.WorkUnit;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.logging.Logger;
import jenkins.model.Jenkins;
import net.jcip.annotations.GuardedBy;

/**
* A {@link Queue.QueueDecisionHandler} that blocks items being deleted from entering the queue.
*
* @see AbstractItem#delete()
* @since 2.55
*/
@Extension
public class ItemDeletion extends Queue.QueueDecisionHandler {

private static final Logger LOGGER = Logger.getLogger(ItemDeletion.class.getName());

/**
* Lock to guard the {@link #registrations} set.
*/
Expand Down Expand Up @@ -176,4 +193,94 @@ public boolean shouldSchedule(Queue.Task p, List<Action> actions) {
}
return true;
}

/**
* Cancels any builds in progress of this item (if a job) or descendants (if a folder).
* Also cancels any associated queue items.
* @param initiatingItem an item being deleted
* @since TODO
*/
public static void cancelBuildsInProgress(@NonNull Item initiatingItem) throws Failure, InterruptedException {
Queue queue = Queue.getInstance();
if (initiatingItem instanceof Queue.Task) {
// clear any items in the queue so they do not get picked up
queue.cancel((Queue.Task) initiatingItem);
}
// now cancel any child items - this happens after ItemDeletion registration, so we can use a snapshot
for (Queue.Item i : queue.getItems()) {
Item item = Tasks.getItemOf(i.task);
while (item != null) {
if (item == initiatingItem) {
if (!queue.cancel(i)) {
LOGGER.warning(() -> "failed to cancel " + i);
}
break;
}
if (item.getParent() instanceof Item) {
item = (Item) item.getParent();
} else {
break;
}
}
}
// interrupt any builds in progress (and this should be a recursive test so that folders do not pay
// the 15 second delay for every child item). This happens after queue cancellation, so will be
// a complete set of builds in flight
Map<Executor, Queue.Executable> buildsInProgress = new LinkedHashMap<>();
for (Computer c : Jenkins.get().getComputers()) {
for (Executor e : c.getAllExecutors()) {
final WorkUnit workUnit = e.getCurrentWorkUnit();
final Queue.Executable executable = workUnit != null ? workUnit.getExecutable() : null;
final SubTask subtask = executable != null ? Executables.getParentOf(executable) : null;
if (subtask != null) {
Item item = Tasks.getItemOf(subtask);
while (item != null) {
if (item == initiatingItem) {
buildsInProgress.put(e, e.getCurrentExecutable());
e.interrupt(Result.ABORTED);
break;
}
if (item.getParent() instanceof Item) {
item = (Item) item.getParent();
} else {
break;
}
}
}
}
}
if (!buildsInProgress.isEmpty()) {
// give them 15 seconds or so to respond to the interrupt
long expiration = System.nanoTime() + TimeUnit.SECONDS.toNanos(15);
// comparison with executor.getCurrentExecutable() == computation currently should always be true
// as we no longer recycle Executors, but safer to future-proof in case we ever revisit recycling
while (!buildsInProgress.isEmpty() && expiration - System.nanoTime() > 0L) {
// we know that ItemDeletion will prevent any new builds in the queue
// ItemDeletion happens-before Queue.cancel so we know that the Queue will stay clear
// Queue.cancel happens-before collecting the buildsInProgress list
// thus buildsInProgress contains the complete set we need to interrupt and wait for
for (Iterator<Map.Entry<Executor, Queue.Executable>> iterator =
buildsInProgress.entrySet().iterator();
iterator.hasNext(); ) {
Map.Entry<Executor, Queue.Executable> entry = iterator.next();
// comparison with executor.getCurrentExecutable() == executable currently should always be
// true as we no longer recycle Executors, but safer to future-proof in case we ever
// revisit recycling.
if (!entry.getKey().isAlive()
|| entry.getValue() != entry.getKey().getCurrentExecutable()) {
iterator.remove();
}
// I don't know why, but we have to keep interrupting
entry.getKey().interrupt(Result.ABORTED);
}
Thread.sleep(50L);
}
if (!buildsInProgress.isEmpty()) {
throw new Failure(Messages.AbstractItem_FailureToStopBuilds(
buildsInProgress.size(), initiatingItem.getFullDisplayName()
));
}
}
}

}

0 comments on commit 220165f

Please sign in to comment.