Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-21044] - Throttling blocks the Jenkins queue
Seems the issue was in improper usage of WeakHashMap (see analysis from @Centic).
I've managed to reproduce the behavior in the following case:
* There is a big number of jobs/configurations with throttling
* The builds queue is not empty
// Seems that ThrottleQueueTaskDispatcher tries to access the data before the complete loading of the plugin's configuration.

This fix provides an explicit locking of any load operations + manual cleanup of erroneous cache data, which goes to persistence in 1.8.1
Resolves https://issues.jenkins-ci.org/browse/JENKINS-21044

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
  • Loading branch information
oleg-nenashev committed Feb 22, 2014
1 parent dc16282 commit 9b7562d
Showing 1 changed file with 20 additions and 10 deletions.
Expand Up @@ -91,13 +91,13 @@ public Object readResolve() {
@Override protected void setOwner(AbstractProject<?,?> owner) {
super.setOwner(owner);
if (throttleEnabled && categories != null) {
Map<String,Map<ThrottleJobProperty,Void>> propertiesByCategory = ((DescriptorImpl) getDescriptor()).propertiesByCategory;
synchronized (propertiesByCategory) {
DescriptorImpl descriptor = (DescriptorImpl) getDescriptor();
synchronized (descriptor.propertiesByCategoryLock) {
for (String c : categories) {
Map<ThrottleJobProperty,Void> properties = propertiesByCategory.get(c);
Map<ThrottleJobProperty,Void> properties = descriptor.propertiesByCategory.get(c);
if (properties == null) {
properties = new WeakHashMap<ThrottleJobProperty,Void>();
propertiesByCategory.put(c, properties);
descriptor.propertiesByCategory.put(c, properties);
}
properties.put(this, null);
}
Expand Down Expand Up @@ -135,9 +135,9 @@ static List<AbstractProject<?,?>> getCategoryProjects(String category) {
assert category != null && !category.equals("");
List<AbstractProject<?,?>> categoryProjects = new ArrayList<AbstractProject<?, ?>>();
Collection<ThrottleJobProperty> properties;
Map<String,Map<ThrottleJobProperty,Void>> propertiesByCategory = Jenkins.getInstance().getDescriptorByType(DescriptorImpl.class).propertiesByCategory;
synchronized (propertiesByCategory) {
Map<ThrottleJobProperty,Void> _properties = propertiesByCategory.get(category);
DescriptorImpl descriptor = Jenkins.getInstance().getDescriptorByType(DescriptorImpl.class);
synchronized (descriptor.propertiesByCategoryLock) {
Map<ThrottleJobProperty,Void> _properties = descriptor.propertiesByCategory.get(category);
properties = _properties != null ? new ArrayList<ThrottleJobProperty>(_properties.keySet()) : Collections.<ThrottleJobProperty>emptySet();
}
for (ThrottleJobProperty t : properties) {
Expand Down Expand Up @@ -165,13 +165,23 @@ public static final class DescriptorImpl extends JobPropertyDescriptor {
private List<ThrottleCategory> categories;

/** Map from category names, to properties including that category. */
private Map<String,Map<ThrottleJobProperty,Void>> propertiesByCategory = new HashMap<String,Map<ThrottleJobProperty,Void>>();
private transient Map<String,Map<ThrottleJobProperty,Void>> propertiesByCategory
= new HashMap<String,Map<ThrottleJobProperty,Void>>();
/** A sync object for {@link #propertiesByCategory} */
private final transient Object propertiesByCategoryLock = new Object();

public DescriptorImpl() {
super(ThrottleJobProperty.class);
load();
synchronized(propertiesByCategoryLock) {
load();
// Explictly drop queue items loaded from 1.8.1 version
if (!propertiesByCategory.isEmpty()) {
propertiesByCategory = new HashMap<String,Map<ThrottleJobProperty,Void>>();
save(); // Save the configuration to remove obsolete data
}
}
}

@Override
public String getDisplayName() {
return "Throttle Concurrent Builds";
Expand Down

0 comments on commit 9b7562d

Please sign in to comment.