Skip to content
Permalink
Browse files

Merge pull request #63 from jglick/hang-JENKINS-34939

[JENKINS-34939] Hold the lock on this folder only while deleting itself, not its children
  • Loading branch information...
jglick committed May 25, 2016
2 parents 645d3c1 + a88aa7c commit 9987689fe62ad7f9e85c685adf1867efc9dff845
@@ -609,9 +609,13 @@ public void onDeleted(TopLevelItem item) throws IOException {
}

@Override
protected void performDelete() throws IOException, InterruptedException {
public void delete() throws IOException, InterruptedException {
// Some parts copied from AbstractItem.
checkPermission(DELETE);
// delete individual items first
// (disregard whether they would be deletable in isolation)
// JENKINS-34939: do not hold the monitor on this folder while deleting them
// (thus we cannot do this inside performDelete)
SecurityContext orig = ACL.impersonate(ACL.SYSTEM);
try {
for (Item i : new ArrayList<Item>(items.values())) {
@@ -627,7 +631,11 @@ protected void performDelete() throws IOException, InterruptedException {
} finally {
SecurityContextHolder.setContext(orig);
}
super.performDelete();
synchronized (this) {
performDelete();
}
getParent().onDeleted(AbstractFolder.this);
Jenkins.getActiveInstance().rebuildDependencyGraphAsync();
}

@Override
@@ -28,12 +28,14 @@
import com.gargoylesoftware.htmlunit.html.HtmlInput;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import com.gargoylesoftware.htmlunit.html.HtmlRadioButtonInput;
import hudson.model.Actionable;
import hudson.model.FreeStyleBuild;
import hudson.model.FreeStyleProject;
import hudson.model.Item;
import hudson.model.Job;
import hudson.model.ListView;
import hudson.model.User;
import hudson.model.listeners.ItemListener;
import hudson.search.SearchItem;
import hudson.security.ACL;
import hudson.security.WhoAmI;
@@ -46,14 +48,22 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.concurrent.Callable;
import jenkins.model.Jenkins;
import jenkins.util.Timer;
import org.acegisecurity.AccessDeniedException;
import static org.junit.Assert.*;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.MockAuthorizationStrategy;
import org.jvnet.hudson.test.SleepBuilder;
import org.jvnet.hudson.test.TestExtension;
import org.jvnet.hudson.test.recipes.LocalData;

public class FolderTest {
@@ -153,6 +163,40 @@ private void copyFromGUI(Folder f, JenkinsRule.WebClient wc, String fromName, St
assertTrue(n.getItem("child2") instanceof FreeStyleProject);
}

@Issue("JENKINS-34939")
@Test public void delete() throws Exception {
Folder d1 = r.jenkins.createProject(Folder.class, "d1");
d1.createProject(FreeStyleProject.class, "p1");
d1.createProject(FreeStyleProject.class, "p2");
d1.createProject(Folder.class, "d2").createProject(FreeStyleProject.class, "p4");
d1.delete();
assertEquals("AbstractFolder.items is sorted by name so we can predict deletion order",
"{d1=[d1], d1/d2=[d1, d1/d2, d1/p1, d1/p2], d1/d2/p4=[d1, d1/d2, d1/d2/p4, d1/p1, d1/p2], d1/p1=[d1, d1/p1, d1/p2], d1/p2=[d1, d1/p2]}",
DeleteListener.whatRemainedWhenDeleted.toString());
}
@TestExtension("delete") public static class DeleteListener extends ItemListener {
static Map<String,Set<String>> whatRemainedWhenDeleted = new TreeMap<String,Set<String>>();
@Override public void onDeleted(Item item) {
try {
// Access metadata from another thread.
whatRemainedWhenDeleted.put(item.getFullName(), Timer.get().submit(new Callable<Set<String>>() {
@Override public Set<String> call() throws Exception {
Set<String> remaining = new TreeSet<String>();
for (Item i : Jenkins.getActiveInstance().getAllItems()) {
remaining.add(i.getFullName());
if (i instanceof Actionable) {
((Actionable) i).getAllActions();
}
}
return remaining;
}
}).get());
} catch (Exception x) {
assert false : x;
}
}
}

/**
* This is more of a test of the core, but make sure the triggers resolve between ourselves.
*/

0 comments on commit 9987689

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