Skip to content

Commit

Permalink
[JENKINS-29936] when removing an item use ACL.SYTEM.
Browse files Browse the repository at this point in the history
The OldDataMonitor should be using ACL.system not the ACL of the calling
thread - this also avoids the deadlock when an authorization strategy is
being saved (locking the auth strategy) which will call into the ODM at
the same point the ODM is being called an a Run has been saved (which will
cause a lookup of the job which will do a permissions check).
  • Loading branch information
jtnord committed Aug 13, 2015
1 parent 6bfbe0c commit 8a077a8
Showing 1 changed file with 23 additions and 6 deletions.
29 changes: 23 additions & 6 deletions core/src/main/java/hudson/diagnosis/OldDataMonitor.java
Expand Up @@ -25,6 +25,7 @@

import com.google.common.base.Predicate;
import com.thoughtworks.xstream.converters.UnmarshallingContext;

import hudson.Extension;
import hudson.XmlFile;
import hudson.model.AdministrativeMonitor;
Expand All @@ -36,8 +37,10 @@
import hudson.model.listeners.ItemListener;
import hudson.model.listeners.RunListener;
import hudson.model.listeners.SaveableListener;
import hudson.security.ACL;
import hudson.util.RobustReflectionConverter;
import hudson.util.VersionNumber;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -48,8 +51,13 @@
import java.util.TreeSet;
import java.util.logging.Level;
import java.util.logging.Logger;

import javax.annotation.CheckForNull;

import jenkins.model.Jenkins;

import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;
import org.kohsuke.stapler.HttpRedirect;
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.HttpResponses;
Expand Down Expand Up @@ -102,12 +110,21 @@ public Map<Saveable,VersionRange> getData() {
}

private static void remove(Saveable obj, boolean isDelete) {
OldDataMonitor odm = get(Jenkins.getInstance());
synchronized (odm) {
odm.data.remove(referTo(obj));
if (isDelete && obj instanceof Job<?,?>)
for (Run r : ((Job<?,?>)obj).getBuilds())
odm.data.remove(referTo(r));
Jenkins j = Jenkins.getInstance();
if (j != null) {
OldDataMonitor odm = get(j);
SecurityContext oldContext = ACL.impersonate(ACL.SYSTEM);
try {
synchronized (odm) {
odm.data.remove(referTo(obj));
if (isDelete && obj instanceof Job<?,?>)
for (Run r : ((Job<?,?>)obj).getBuilds())
odm.data.remove(referTo(r));
}
}
finally {
SecurityContextHolder.setContext(oldContext);
}
}
}

Expand Down

0 comments on commit 8a077a8

Please sign in to comment.