Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-36372

After resuming build, newly loaded scripts do not share root binding

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Major
    • Resolution: Fixed
    • Component/s: pipeline
    • Labels:
      None
    • Similar Issues:

      Description

      Normally a Binding from the main CpsScript is shared with those from scripts created via load, because InvokerHelper.createScript links them to the binding defined in CpsGroovyShell.context.

      After the build is resumed from disk, each time SerializableScript.readResolve is called, it uses a distinct Binding with the same variables as in the original program. So the main script and any scripts already loaded are OK, or at least have the same bindings as they did before; presumably subsequent changes to the main script's bindings would no longer be visible to loaded scripts. The more visible problem is that the shell's binding is now empty (because it is a fresh instance from CpsFlowExecution.parseScript), so the next time load is called, that new script gets a clone of an empty binding—so it only gets a DSL bound via CpsScript.$initialize, nothing more.

      Making SerializableScript save the original Binding to the stream is not an option, since Binding is not Serializable.

      You would think that reparse could somehow track the bindings in the Script it returns, but in fact this instance is discarded—only its Class is used, since the actual instance is produced by deserialization.

      You might then think that CpsFlowExecution.loadProgramAsync could make sure that the context of the shell got linked to the binding of the main script (and ideally the same for previously loaded scripts, too). But after deserialization, we only have a CpsThreadGroup, and it is not obvious how to find the CpsScript instance(s) associated with it: they are held implicitly somewhere in the Continuation.

      Kohsuke Kawaguchi help!

        Attachments

          Issue Links

            Activity

            Hide
            jglick Jesse Glick added a comment -

            they are held implicitly somewhere

            Specifically:

            this     - value: WorkflowScript #1
             <- value     - class: java.util.HashMap$Node, value: WorkflowScript #1
              <- [8]     - class: java.util.HashMap$Node[], value: java.util.HashMap$Node #36691
               <- table     - class: java.util.HashMap, value: java.util.HashMap$Node[] #11010
                <- locals     - class: com.cloudbees.groovy.cps.impl.FunctionCallEnv, value: java.util.HashMap #10890
                 <- parent     - class: com.cloudbees.groovy.cps.impl.BlockScopeEnv, value: com.cloudbees.groovy.cps.impl.FunctionCallEnv #2
                  <- e     - class: com.cloudbees.groovy.cps.impl.SequenceBlock$ContinuationImpl, value: com.cloudbees.groovy.cps.impl.BlockScopeEnv #1
                   <- k     - class: com.cloudbees.groovy.cps.Continuable, value: com.cloudbees.groovy.cps.impl.SequenceBlock$ContinuationImpl #1
                    <- program     - class: org.jenkinsci.plugins.workflow.cps.CpsThread, value: com.cloudbees.groovy.cps.Continuable #1
                     <- value     - class: java.util.TreeMap$Entry, value: org.jenkinsci.plugins.workflow.cps.CpsThread #1
                      <- root     - class: java.util.TreeMap, value: java.util.TreeMap$Entry #1434
                       <- threads     - class: org.jenkinsci.plugins.workflow.cps.CpsThreadGroup, value: java.util.TreeMap #1363
            
            Show
            jglick Jesse Glick added a comment - they are held implicitly somewhere Specifically: this - value: WorkflowScript #1 <- value - class: java.util.HashMap$Node, value: WorkflowScript #1 <- [8] - class: java.util.HashMap$Node[], value: java.util.HashMap$Node #36691 <- table - class: java.util.HashMap, value: java.util.HashMap$Node[] #11010 <- locals - class: com.cloudbees.groovy.cps.impl.FunctionCallEnv, value: java.util.HashMap #10890 <- parent - class: com.cloudbees.groovy.cps.impl.BlockScopeEnv, value: com.cloudbees.groovy.cps.impl.FunctionCallEnv #2 <- e - class: com.cloudbees.groovy.cps.impl.SequenceBlock$ContinuationImpl, value: com.cloudbees.groovy.cps.impl.BlockScopeEnv #1 <- k - class: com.cloudbees.groovy.cps.Continuable, value: com.cloudbees.groovy.cps.impl.SequenceBlock$ContinuationImpl #1 <- program - class: org.jenkinsci.plugins.workflow.cps.CpsThread, value: com.cloudbees.groovy.cps.Continuable #1 <- value - class: java.util.TreeMap$Entry, value: org.jenkinsci.plugins.workflow.cps.CpsThread #1 <- root - class: java.util.TreeMap, value: java.util.TreeMap$Entry #1434 <- threads - class: org.jenkinsci.plugins.workflow.cps.CpsThreadGroup, value: java.util.TreeMap #1363
            Hide
            jglick Jesse Glick added a comment -

            Seems this can be fixed by explicitly storing scripts in the CpsThreadGroup.

            workflow-cps-global-lib does not seem to be affected, since apparently those scripts do not share a Binding with the main script to begin with. I do not exactly understand why not, but at any rate I see no reason to try to support sharing of root bindings here if no one is clamoring for it; better to pass in variables explicitly when you need them.

            Show
            jglick Jesse Glick added a comment - Seems this can be fixed by explicitly storing scripts in the CpsThreadGroup . workflow-cps-global-lib does not seem to be affected, since apparently those scripts do not share a Binding with the main script to begin with. I do not exactly understand why not, but at any rate I see no reason to try to support sharing of root bindings here if no one is clamoring for it; better to pass in variables explicitly when you need them.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Jesse Glick
            Path:
            src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java
            http://jenkins-ci.org/commit/workflow-cps-plugin/c95585b6e1e3d87c7fce1ed8f8394aeb8fdb1f88
            Log:
            FindBugs noticed a possible NPE in the original #29.
            Theoretically could be thrown if a build started prior to the fix of JENKINS-36372 `load`ed something after the upgrade restart.

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java http://jenkins-ci.org/commit/workflow-cps-plugin/c95585b6e1e3d87c7fce1ed8f8394aeb8fdb1f88 Log: FindBugs noticed a possible NPE in the original #29. Theoretically could be thrown if a build started prior to the fix of JENKINS-36372 `load`ed something after the upgrade restart.

              People

              • Assignee:
                jglick Jesse Glick
                Reporter:
                jglick Jesse Glick
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: