[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.) Originally-Committed-As: 6140881717bf937ecb42bf77ec11dcc208bedd0c
jenkinsci · Sep 27, 2015 · 16131b6 · 16131b6
1 parent b6d0311
commit 16131b6
Showing 1 changed file with 15 additions and 13 deletions.
diff --git a/job/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java b/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;
             }
         }