Branch: refs/heads/synchronous-abnormal-body-execution
  Home:   https://github.com/jenkinsci/workflow-cps-plugin
  Commit: 0e2a049ce635a71a3b341ecf959350c628675ae9
      
https://github.com/jenkinsci/workflow-cps-plugin/commit/0e2a049ce635a71a3b341ecf959350c628675ae9
  Author: Kohsuke Kawaguchi <k...@kohsuke.org>
  Date:   2016-09-19 (Mon, 19 Sep 2016)

  Changed paths:
    A src/test/java/org/jenkinsci/plugins/workflow/cps/CpsBodyExecutionTest.java

  Log Message:
  -----------
  Test case that reproduces the problem


  Commit: ac9fe50fc2a26f697e49714c05254d50245fe99e
      
https://github.com/jenkinsci/workflow-cps-plugin/commit/ac9fe50fc2a26f697e49714c05254d50245fe99e
  Author: Kohsuke Kawaguchi <k...@kohsuke.org>
  Date:   2016-09-19 (Mon, 19 Sep 2016)

  Changed paths:
    M src/main/java/org/jenkinsci/plugins/workflow/cps/CpsBodyExecution.java
    M src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThread.java

  Log Message:
  -----------
  Fixing a bug in CPS VM about how it handles an exception in sync body.

See the previous commit for the test case that reproduces this problem.

Any exception aside from CpsCallableInvocation is a sign that the
execution has completed abnormally and synchronously. So this code pass
should have close parallel to the normal case that immediately follows
the 'params.body.getBody(currentThread).call()' invocation, yet the
thread association is missing from the failure case.

CpsBodyExecution.onSuccess/onFailure also assumes that they run in
CPS VM thread to be able to grow the flow node graph, as can be seen
in their use of CpsThread.current(). In the typical case where the body
being invoked is also CPS code, this assumption correctly holds as these
callbacks as invoked as Continuation while running CPS thread code, but
if it executes synchronously, this assumption does not hold, as the call
stack originates in `ThreadTask.eval(this)` that's outside the code
that sets the CpsThread.current() return value.

ThreadTask.eval() always operate in the context of single CpsThread,
it seems reasonable to run this code whiel CpsThread.current() is still
set.

(bug that motivated this change, from the parent of this commit)
------------------------------
When the body of a step is synchronous and explodes, the failure should
be recorded and the pipeline job should move on.

But instead, this hangs because CpsBodyExecution has a bug in how it
handles this case. It tries to launch the body (in this case the
'bodyBlock' method) in a separate CPS thread, and puts the parent CPS
thread on hold. Yet when the child CPS thread ends with an exception, it
fails to record this result correctly, and it gets into the eternal
sleep in which the parent CPS thread expects to be notified of the
outcome of the child CPS thread, which never arrives.


  Commit: 701f72334faa95fcac88c30447a67853e29263f0
      
https://github.com/jenkinsci/workflow-cps-plugin/commit/701f72334faa95fcac88c30447a67853e29263f0
  Author: Kohsuke Kawaguchi <k...@kohsuke.org>
  Date:   2016-09-19 (Mon, 19 Sep 2016)

  Changed paths:
    M src/test/java/org/jenkinsci/plugins/workflow/cps/CpsBodyExecutionTest.java
    A src/test/java/org/jenkinsci/plugins/workflow/cps/EmperorHasNoClothes.java

  Log Message:
  -----------
  Expanded the assertions

Test the shape of flow node graph and make sure the exception is
properly recorded.


Compare: 
https://github.com/jenkinsci/workflow-cps-plugin/compare/0e2a049ce635^...701f72334faa

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Commits" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-commits+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to