Fix more subtle durability issues

pom.xml

@@ -142,7 +142,7 @@
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-job</artifactId>
             <!-- Snapshot for better handling of onLoad errors -->
-            <version>2.21-20180427.223321-5</version>
+            <version>2.21-20180501.222458-6</version>
             <scope>test</scope>
         </dependency>
         <dependency>

src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java

@@ -620,10 +620,11 @@ private synchronized String getHeadsAsString() {
      * Bypasses {@link #croak(Throwable)} and {@link #onProgramEnd(Outcome)} to guarantee a clean path.
      */
     @GuardedBy("this")
-    void createPlaceholderNodes(Throwable failureReason) throws Exception {
+    synchronized void createPlaceholderNodes(Throwable failureReason) throws Exception {
         this.done = true;
 
         if (this.owner != null) {
+            // Ensure that the Run is marked as completed (failed) if it isn't already so it won't show as running
             Queue.Executable ex = owner.getExecutable();
             if (ex instanceof Run) {
                 Result res = ((Run)ex).getResult();
@@ -729,10 +730,10 @@ public void onLoad(FlowExecutionOwner owner) throws IOException {
         try {
             if (isComplete()) {
                 if (done == Boolean.TRUE && !super.isComplete()) {
-                    LOGGER.log(Level.WARNING, "Completed flow without FlowEndNode: "+this+" heads:"+getHeadsAsString());
+                    LOGGER.log(Level.INFO, "Completed flow without FlowEndNode: "+this+" heads:"+getHeadsAsString());
                 }
                 if (super.isComplete() && done != Boolean.TRUE) {
-                    LOGGER.log(Level.WARNING, "Flow has FlowEndNode, but is not marked as done, fixing this for"+this);
+                    LOGGER.log(Level.FINE, "Flow has FlowEndNode, but is not marked as done, fixing this for"+this);
                     done = true;
                     saveOwner();
                 }
@@ -746,7 +747,7 @@ public void onLoad(FlowExecutionOwner owner) throws IOException {
                     throw new IOException("Cannot resume build -- was not cleanly saved when Jenkins shut down.");
                 }
             }
-        } catch (Exception e) {  // Multicatch ensures that failure to load does not nuke the master
+        } catch (Exception e) {  // Broad catch ensures that failure to load do NOT nuke the master
             SettableFuture<CpsThreadGroup> p = SettableFuture.create();
             programPromise = p;
             loadProgramFailed(e, p);
@@ -930,7 +931,7 @@ public void onFailure(Throwable t) {
         try {
             g = programPromise.get();
         } catch (Exception x) {
-            // FIXME check this won't cause issues due to depickling delays etc
+            // TODO Check this won't cause issues due to depickling delays etc
             LOGGER.log(Level.FINE, "Not blocking restart due to exception in ProgramPromise: "+this, x);
             return false;
         }
@@ -1228,7 +1229,6 @@ synchronized void onProgramEnd(Outcome outcome) {
         // shrink everything into a single new head
         try {
             if (heads != null) {
-                // Below does not look correct to me
                 FlowHead first = getFirstHead();
                 first.setNewHead(head);
                 done = true;  // After setting the final head

src/main/java/org/jenkinsci/plugins/workflow/cps/DSL.java

@@ -578,7 +578,7 @@ private void invokeBody(CpsThread cur) {
             // we want to do this first before starting body so that the order of heads preserve
             // natural ordering.
 
-            // FIXME give this javadocs worth a darn, because this is how we create parallel branches and the docs are crypic.
+            // TODO give this javadocs worth a darn, because this is how we create parallel branches and the docs are cryptic as can be!
             // Also we need to double-check this logic because this might cause a failure of persistence
             FlowHead[] heads = new FlowHead[context.bodyInvokers.size()];
             for (int i = 0; i < heads.length; i++) {

src/test/java/org/jenkinsci/plugins/workflow/cps/FlowDurabilityTest.java

@@ -794,11 +794,7 @@ public void evaluate() throws Throwable {
 
     private static void assertBuildNotHung(@Nonnull RestartableJenkinsRule story, @Nonnull  WorkflowRun run, int timeOutMillis) throws Exception {
         if (run.isBuilding()) {
-            try {
-                story.j.waitUntilNoActivityUpTo(timeOutMillis);
-            } catch (AssertionError ase) {  // Allows attaching a debugger here
-                throw new AssertionError("Build hung: " + run, ase);
-            }
+            story.j.waitUntilNoActivityUpTo(timeOutMillis);
         }
     }
 
@@ -877,7 +873,7 @@ public void evaluate() throws Throwable {
             @Override
             public void evaluate() throws Throwable {
                 WorkflowRun run = story.j.jenkins.getItemByFullName(jobName, WorkflowJob.class).getLastBuild();
-                if (run == null) {
+                if (run == null) {   // Build killed so early the Run did not get to persist
                     return;
                 }
                 if (run.getExecution() != null) {
@@ -925,7 +921,7 @@ public void evaluate() throws Throwable {
             @Override
             public void evaluate() throws Throwable {
                 WorkflowRun run = story.j.jenkins.getItemByFullName(jobName, WorkflowJob.class).getLastBuild();
-                if (run == null) {
+                if (run == null) {   // Build killed so early the Run did not get to persist
                     return;
                 }
                 if (run.getExecution() != null) {
@@ -983,7 +979,7 @@ public void evaluate() throws Throwable {
             @Override
             public void evaluate() throws Throwable {
                 WorkflowRun run = story.j.jenkins.getItemByFullName(jobName, WorkflowJob.class).getLastBuild();
-                if (run == null) {
+                if (run == null) {   // Build killed so early the Run did not get to persist
                     return;
                 }
                 if (run.getExecution() != null) {