Skip to content
Permalink
Browse files

[FIXED JENKINS-27057] Made logsToCopy a concurrent map, since Workflo…

…wRun.save can be called from various threads.

The mutations are all guarded by WorkflowRun.completed, so we just need to allow serialization to see a snapshot.
(There was already a race condition in the case of abrupt shutdown, but at worst these should result in duplicated log text.)
  • Loading branch information
jglick committed Sep 27, 2015
1 parent 815afe3 commit 6140881717bf937ecb42bf77ec11dcc208bedd0c
Showing with 15 additions and 13 deletions.
  1. +15 −13 job/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java
@@ -59,11 +59,11 @@
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.WeakHashMap;
import java.util.concurrent.ConcurrentSkipListMap;
import java.util.concurrent.Future;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
@@ -185,7 +185,7 @@ public WorkflowRun(WorkflowJob job, File dir) throws IOException {
FlowExecutionList.get().register(owner);
newExecution.addListener(new GraphL(), false);
completed = new AtomicBoolean();
logsToCopy = new LinkedHashMap<String,Long>();
logsToCopy = new ConcurrentSkipListMap<String,Long>();
execution = newExecution;
newExecution.start();
executionPromise.set(newExecution);
@@ -247,22 +247,24 @@ private void copyLogs() {
if (logsToCopy == null) { // finished
return;
}
Iterator<Map.Entry<String,Long>> it = logsToCopy.entrySet().iterator();
if (logsToCopy instanceof LinkedHashMap) { // upgrade while build is running
logsToCopy = new ConcurrentSkipListMap<String,Long>(logsToCopy);
}
boolean modified = false;
while (it.hasNext()) {
Map.Entry<String,Long> entry = it.next();
for (Map.Entry<String,Long> entry : logsToCopy.entrySet()) {
String id = entry.getKey();
FlowNode node;
try {
node = execution.getNode(entry.getKey());
node = execution.getNode(id);
} catch (IOException x) {
LOGGER.log(Level.WARNING, null, x);
it.remove();
logsToCopy.remove(id);
modified = true;
continue;
}
if (node == null) {
LOGGER.log(Level.WARNING, "no such node {0}", entry.getKey());
it.remove();
LOGGER.log(Level.WARNING, "no such node {0}", id);
logsToCopy.remove(id);
modified = true;
continue;
}
@@ -283,13 +285,13 @@ private void copyLogs() {
try {
long revised = logText.writeRawLogTo(old, logger);
if (revised != old) {
entry.setValue(revised);
logsToCopy.put(id, revised);
modified = true;
}
if (logText.isComplete()) {
logText.writeRawLogTo(entry.getValue(), logger); // defend against race condition?
assert !node.isRunning() : "LargeText.complete yet " + node + " claims to still be running";
it.remove();
logsToCopy.remove(id);
modified = true;
}
} finally {
@@ -299,11 +301,11 @@ private void copyLogs() {
}
} catch (IOException x) {
LOGGER.log(Level.WARNING, null, x);
it.remove();
logsToCopy.remove(id);
modified = true;
}
} else if (!node.isRunning()) {
it.remove();
logsToCopy.remove(id);
modified = true;
}
}

0 comments on commit 6140881

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