Skip to content
Permalink
Browse files

[FIXED JENKINS-18589]

Use DescribableList to handle the copy-on-write semantics correctly. The vector class just doesn't cut it, and we've been setting a new value to this field, which will violates all sorts of the concurrent programming practice.

This change has the nice side effect of removing {{class="vector"}} from the persisted XML. A test is added to make sure we can still read back such an XML.

(cherry picked from commit 7facc77)

Conflicts:
	test/src/test/groovy/hudson/model/AbstractProjectTest.groovy
  • Loading branch information...
kohsuke authored and olivergondza committed Jul 2, 2013
1 parent 68bde83 commit 5407d9fbce6b10d6902f0cc5971ee95c71619f3a
@@ -55,6 +55,9 @@
<!-- Record your changes in the trunk here. -->
<div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
<ul class=image>
<li class=bug>
Fixed a dead lock in the <tt>Project</tt> class and improved the signature of the persisted XML form a bit.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-18589">issue 18589</a>)
<li class=bug>
Improved memory efficiency in parsing test reports with large stdio output files.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-15382">issue 15382</a>)
@@ -418,7 +418,7 @@ public void setTouchStoneResultCondition(Result touchStoneResultCondition) throw
r.addAll(step.getProjectActions(this));
for (BuildWrapper step : buildWrappers)
r.addAll(step.getProjectActions(this));
for (Trigger<?> trigger : triggers)
for (Trigger<?> trigger : triggers())
r.addAll(trigger.getProjectActions());

return r;
@@ -125,6 +125,7 @@
import java.util.TreeMap;
import java.util.Vector;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
import java.util.logging.Level;
import java.util.logging.Logger;

@@ -234,7 +235,9 @@
/**
* List of all {@link Trigger}s for this project.
*/
protected List<Trigger<?>> triggers = new Vector<Trigger<?>>();
protected volatile DescribableList<Trigger<?>,TriggerDescriptor> triggers = new DescribableList<Trigger<?>,TriggerDescriptor>(this);
private AtomicReferenceFieldUpdater<AbstractProject,List> triggersUpdater
= AtomicReferenceFieldUpdater.newUpdater(AbstractProject.class,List.class,"triggers");

/**
* {@link Action}s contributed from subsidiary objects associated with
@@ -303,6 +306,7 @@ public void onLoad(ItemGroup<? extends Item> parent, String name) throws IOExcep
}
}
this.builds = builds;
triggers().setOwner(this);
for (Trigger t : triggers())
t.start(this, Items.updatingByXml.get());
if(scm==null)
@@ -321,9 +325,10 @@ public R create(File dir) throws IOException {
});
}

private synchronized List<Trigger<?>> triggers() {
@WithBridgeMethods(List.class)
protected DescribableList<Trigger<?>,TriggerDescriptor> triggers() {
if (triggers == null) {
triggers = new Vector<Trigger<?>>();
triggersUpdater.compareAndSet(this,null,new DescribableList<Trigger<?>,TriggerDescriptor>(this));
}
return triggers;
}
@@ -1634,8 +1639,8 @@ void removeFromList(Descriptor<T> item, List<T> collection) throws IOException {
}

@SuppressWarnings("unchecked")
public synchronized Map<TriggerDescriptor,Trigger> getTriggers() {
return (Map)Descriptor.toMap(triggers());
public Map<TriggerDescriptor,Trigger<?>> getTriggers() {
return triggers().toMap();
}

/**
@@ -1959,8 +1964,8 @@ protected void submit(StaplerRequest req, StaplerResponse rsp) throws IOExceptio

for (Trigger t : triggers())
t.stop();
triggers = buildDescribable(req, Trigger.for_(this));
for (Trigger t : triggers)
triggers.replaceBy(buildDescribable(req, Trigger.for_(this)));
for (Trigger t : triggers())
t.start(this,true);

for (Publisher _t : Descriptor.newInstancesFromHeteroList(req, json, "publisher", Jenkins.getInstance().getExtensionList(BuildTrigger.DescriptorImpl.class))) {
@@ -47,6 +47,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;

/**
* Buildable software project.
@@ -59,17 +60,23 @@
/**
* List of active {@link Builder}s configured for this project.
*/
private DescribableList<Builder,Descriptor<Builder>> builders;
private volatile DescribableList<Builder,Descriptor<Builder>> builders;
private static final AtomicReferenceFieldUpdater<Project,DescribableList> buildersSetter
= AtomicReferenceFieldUpdater.newUpdater(Project.class,DescribableList.class,"builders");

/**
* List of active {@link Publisher}s configured for this project.
*/
private DescribableList<Publisher,Descriptor<Publisher>> publishers;
private volatile DescribableList<Publisher,Descriptor<Publisher>> publishers;
private static final AtomicReferenceFieldUpdater<Project,DescribableList> publishersSetter
= AtomicReferenceFieldUpdater.newUpdater(Project.class,DescribableList.class,"publishers");

/**
* List of active {@link BuildWrapper}s configured for this project.
*/
private DescribableList<BuildWrapper,Descriptor<BuildWrapper>> buildWrappers;
private volatile DescribableList<BuildWrapper,Descriptor<BuildWrapper>> buildWrappers;
private static final AtomicReferenceFieldUpdater<Project,DescribableList> buildWrappersSetter
= AtomicReferenceFieldUpdater.newUpdater(Project.class,DescribableList.class,"buildWrappers");

/**
* Creates a new project.
@@ -103,16 +110,16 @@ public void onLoad(ItemGroup<? extends Item> parent, String name) throws IOExcep
return getPublishersList().toMap();
}

public synchronized DescribableList<Builder,Descriptor<Builder>> getBuildersList() {
public DescribableList<Builder,Descriptor<Builder>> getBuildersList() {
if (builders == null) {
builders = new DescribableList<Builder,Descriptor<Builder>>(this);
buildersSetter.compareAndSet(this,null,new DescribableList<Builder,Descriptor<Builder>>(this));
}
return builders;
}

public synchronized DescribableList<Publisher,Descriptor<Publisher>> getPublishersList() {
public DescribableList<Publisher,Descriptor<Publisher>> getPublishersList() {
if (publishers == null) {
publishers = new DescribableList<Publisher,Descriptor<Publisher>>(this);
publishersSetter.compareAndSet(this,null,new DescribableList<Publisher,Descriptor<Publisher>>(this));
}
return publishers;
}
@@ -121,9 +128,9 @@ public void onLoad(ItemGroup<? extends Item> parent, String name) throws IOExcep
return getBuildWrappersList().toMap();
}

public synchronized DescribableList<BuildWrapper, Descriptor<BuildWrapper>> getBuildWrappersList() {
public DescribableList<BuildWrapper, Descriptor<BuildWrapper>> getBuildWrappersList() {
if (buildWrappers == null) {
buildWrappers = new DescribableList<BuildWrapper,Descriptor<BuildWrapper>>(this);
buildWrappersSetter.compareAndSet(this,null,new DescribableList<BuildWrapper,Descriptor<BuildWrapper>>(this));
}
return buildWrappers;
}
@@ -211,7 +218,7 @@ protected void submit( StaplerRequest req, StaplerResponse rsp ) throws IOExcept
r.addAll(step.getProjectActions(this));
for (BuildWrapper step : getBuildWrappers().values())
r.addAll(step.getProjectActions(this));
for (Trigger trigger : getTriggers().values())
for (Trigger trigger : triggers())
r.addAll(trigger.getProjectActions());

return r;
@@ -184,7 +184,7 @@ protected AbstractMavenProject(ItemGroup parent, String name) {
addTransientActionsFromBuild(getLastBuild(),r,added);
addTransientActionsFromBuild(getLastSuccessfulBuild(),r,added);

for (Trigger<?> trigger : triggers)
for (Trigger<?> trigger : triggers())
r.addAll(trigger.getProjectActions());

return r;
@@ -26,9 +26,7 @@ package hudson.model;
import com.gargoylesoftware.htmlunit.ElementNotFoundException;
import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import com.gargoylesoftware.htmlunit.HttpMethod;
import com.gargoylesoftware.htmlunit.WebRequestSettings;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlInput;
import com.gargoylesoftware.htmlunit.WebRequestSettings
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import hudson.security.*;
import hudson.tasks.BuildTrigger;
@@ -38,31 +36,21 @@ import hudson.Launcher;
import hudson.FilePath;
import hudson.Functions;
import hudson.Util;
import hudson.tasks.ArtifactArchiver;
import hudson.tasks.ArtifactArchiver
import hudson.triggers.SCMTrigger;
import hudson.util.StreamTaskListener;
import hudson.util.OneShotEvent;
import java.io.IOException;

import hudson.util.OneShotEvent
import jenkins.model.Jenkins;
import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;
import org.jvnet.hudson.test.HudsonTestCase;
import org.jvnet.hudson.test.HudsonTestCase.WebClient;
import org.jvnet.hudson.test.HudsonTestCase
import org.jvnet.hudson.test.Bug;
import org.jvnet.hudson.test.MemoryAssert;
import org.jvnet.hudson.test.recipes.PresetData;
import org.jvnet.hudson.test.recipes.PresetData.DataSet;

import java.io.File;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Future;
import org.jvnet.hudson.test.recipes.PresetData.DataSet
import org.apache.commons.io.FileUtils;
import java.lang.ref.WeakReference;
import java.net.HttpURLConnection;
import java.net.URL;
import java.lang.ref.WeakReference

import org.jvnet.hudson.test.MockFolder;

/**
@@ -0,0 +1,8 @@
<?xml version='1.0' encoding='UTF-8'?>
<project>
<triggers class="vector">
<hudson.triggers.SCMTrigger>
<spec>*/10 * * * *</spec>
</hudson.triggers.SCMTrigger>
</triggers>
</project>

0 comments on commit 5407d9f

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