[FIXED JENKINS-29221] WorkflowRun assigned FlowExecution when executi…

…on start fails Originally-Committed-As: adeaf564a94ee6f82861e4253bd2d5bbc22276c4
jenkinsci · Jul 3, 2015 · 0827836 · 0827836
1 parent 4bdfe82
commit 0827836
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 4 deletions.
diff --git a/aggregator/src/test/java/org/jenkinsci/plugins/workflow/WorkflowRunTest.java b/aggregator/src/test/java/org/jenkinsci/plugins/workflow/WorkflowRunTest.java
@@ -201,6 +201,26 @@ public void contextInjectionOfSubParameters() throws Exception {
         r.assertBuildStatusSuccess(p.scheduleBuild2(0));
     }
 
+    @Test @Issue("JENKINS-29221")
+    public void failedToStartRun() throws Exception {
+        p = r.jenkins.createProject(WorkflowJob.class, "p");
+        p.setDefinition(new CpsFlowDefinition(
+                "{{stage 'dev'\n" +
+                        "def hello = new HelloWorld()\n" +
+                        "public class HelloWorld()\n" +
+                        "{ // <- invalid class definition }\n" +
+                        "}}"
+        ));
+        QueueTaskFuture<WorkflowRun> workflowRunQueueTaskFuture = p.scheduleBuild2(0);
+        WorkflowRun run = r.assertBuildStatus(Result.FAILURE, workflowRunQueueTaskFuture.get());
+
+        // The issue was that the WorkflowRun's execution instance got initialised even though the script
+        // was bad and the execution never started. The fix is to ensure that it only gets initialised
+        // if the execution instance starts successfully i.e. in the case of this test, that it doesn't
+        // get initialised.
+        assertNull(run.getExecution());
+    }
+
     @Issue("JENKINS-27531")
     @LocalData
     @Test public void loadMigratedBuildRecord() throws Exception {

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
@@ -180,13 +180,18 @@ public WorkflowRun(WorkflowJob job, File dir) throws IOException {
             }
             checkouts = new LinkedList<SCMCheckout>();
             Owner owner = new Owner(this);
-            execution = definition.create(owner, listener, getAllActions());
+
+            FlowExecution newExecution = definition.create(owner, listener, getAllActions());
             FlowExecutionList.get().register(owner);
-            execution.addListener(new GraphL());
+            newExecution.addListener(new GraphL());
             completed = new AtomicBoolean();
             logsToCopy = new LinkedHashMap<String,Long>();
-            execution.start();
-            executionPromise.set(execution);
+            newExecution.start();
+
+            // It's only okay to initialise the global once the new FlowExecution
+            // has successfully started.
+            execution = newExecution;
+            executionPromise.set(newExecution);
         } catch (Throwable x) {
             if (listener == null) {
                 LOGGER.log(Level.WARNING, this + " failed to start", x);