Skip to content
Permalink
Browse files

Merge pull request #40 from jenkinsci/JENKINS-48465-fix-concurrent-mo…

…dification-exception

[JENKINS-48465] Fix ConcurrentModificationException in PlotBuildAction
  • Loading branch information
vgaidarji committed Jan 3, 2018
2 parents 490c98c + 0f03011 commit 56fc2182807a04a763844f8bbb1b7ca3109d1d78
@@ -3,10 +3,10 @@
import hudson.model.Action;
import hudson.model.InvisibleAction;
import hudson.model.Run;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import jenkins.tasks.SimpleBuildStep;
import org.kohsuke.stapler.StaplerProxy;
import org.kohsuke.stapler.export.ExportedBean;
@@ -20,11 +20,11 @@

PlotBuildAction(Run<?, ?> run, List<Plot> plots) {
this.run = run;
this.plots = plots;
this.plots = new CopyOnWriteArrayList<>(plots);
}

@Override
public synchronized Collection<? extends Action> getProjectActions() {
public Collection<? extends Action> getProjectActions() {
return Collections.<Action>singleton(new PlotAction(run.getParent(), plots));
}

@@ -33,9 +33,9 @@ public Object getTarget() {
return null;
}

synchronized void addPlots(List<Plot> plots) {
void addPlots(List<Plot> plots) {
if (this.plots == null) {
this.plots = new ArrayList<>();
this.plots = new CopyOnWriteArrayList<>();
}
this.plots.addAll(plots);
}
@@ -0,0 +1,116 @@
package hudson.plugins.plot;

import com.thoughtworks.xstream.converters.reflection.PureJavaReflectionProvider;
import com.thoughtworks.xstream.converters.reflection.ReflectionProvider;
import hudson.model.Run;
import java.util.ArrayList;
import java.util.ConcurrentModificationException;
import java.util.List;
import java.util.Random;
import java.util.concurrent.Callable;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.FutureTask;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;

import static org.junit.Assert.fail;

public class PlotBuildActionTest {

@Rule
public JenkinsRule r = new JenkinsRule();
private PlotBuildAction plotBuildAction;

@Before
public void setUp() throws Exception {
final Run<?, ?> run = r.buildAndAssertSuccess(r.createFreeStyleProject());
final List<Plot> plots = new ArrayList<>();
for (int i = 0; i < 30; i++) {
Plot p = new Plot();
p.title = String.valueOf(i);
plots.add(p);
}
plotBuildAction = new PlotBuildAction(run, plots);
}

@Issue("JENKINS-48465")
@Test
public void checksNoConcurrentModificationExceptionIsThrownForPlotsListAccess()
throws Exception {
int tasksCount = 10;
ExecutorService executorService = Executors.newFixedThreadPool(2);
List<FutureTask<Object>> tasks = new ArrayList<>();
CountDownLatch latch = new CountDownLatch(tasksCount);

simulateConcurrentModificationException(executorService, tasksCount, tasks, latch);

waitForAllThreadsToFinish(executorService, latch);
assertNoConcurrentModificationExceptionThrown(tasks);
}

private void simulateConcurrentModificationException(ExecutorService executorService,
int tasksCount, List<FutureTask<Object>> tasks, final CountDownLatch latch) {
for (int i = 0; i < tasksCount; i++) {
FutureTask<Object> task = new FutureTask<>(new Callable<Object>() {
@Override
public Object call() throws Exception {
try {
Thread.sleep(new Random().nextInt(100));
// using PureJavaReflectionProvider just because it's used in Jenkins
// close to "real world"
PureJavaReflectionProvider provider = new PureJavaReflectionProvider();
provider.visitSerializableFields(plotBuildAction,
new ReflectionProvider.Visitor() {
@Override
public void visit(String fieldName, Class fieldType,
Class definedIn, Object value) {
if (value != null && value instanceof List) {
List<Plot> plots = (List<Plot>) value;
// simulate ConcurrentModificationException
for (Plot p : plots) {
if (plots.size() > 0) {
plots.remove(p);
}
}
}
}
});
} finally {
latch.countDown();
}
return null;
}
});
tasks.add(task);
executorService.submit(task);
}
}

private void waitForAllThreadsToFinish(ExecutorService executorService, CountDownLatch latch) {
try {
latch.await();
} catch (InterruptedException e) {
e.printStackTrace();
}
executorService.shutdown();
}

private void assertNoConcurrentModificationExceptionThrown(List<FutureTask<Object>> tasks)
throws InterruptedException {
try {
// we expect here no ConcurrentModificationException
// otherwise access to plots list is not synchronized
for (FutureTask task : tasks) {
task.get();
}
} catch (ExecutionException | ConcurrentModificationException e) {
fail("Access to PlotBuildAction#plots list is not synchronized");
}
}
}

0 comments on commit 56fc218

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