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

FutureImpl.cancel() doesn't cancel the linked job

    XMLWordPrintable

    Details

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

      Description

      FutureImpl.cancel() doesn't cancel the job linked to the Future object, as executor set is always empty. It's supposed to be filled in WorkUnitContext.createWorkUnit() but it's not executed on the Executor thread, unlike the comment suggests:

          public WorkUnit createWorkUnit(SubTask execUnit) {
              Executor executor = Executor.currentExecutor();
              if (executor != null) { // TODO is it legal for this to be called by a non-executor thread?
                  future.addExecutor(executor);
              }
      

      This breaks build cancellation from Jenkins CLI. When a build is started with -s parameter, it's not canceled when the CLI process is interrupted. The CLI handling code in BuildCommand correctly calls FutureImpl.cancel() but nothing happens, as executor set is empty.

        Attachments

          Activity

          Hide
          danielbeck Daniel Beck added a comment -

          Great report! I always wondered why -s was broken…

          Show
          danielbeck Daniel Beck added a comment - Great report! I always wondered why -s was broken…
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Yoann Dubreuil
          Path:
          core/src/main/java/hudson/model/queue/WorkUnit.java
          core/src/main/java/hudson/model/queue/WorkUnitContext.java
          http://jenkins-ci.org/commit/jenkins/8d3eb20e5d82cf3528849301493ec5b2ddf9dd87
          Log:
          JENKINS-33038 correctly set executor in FutureImpl

          This patch fixes cancellation of the build linked to the Future
          object. Before, the executor set in FutureImpl was always empty,
          because WorkUnitContext.createWorkUnit() is not called from an
          Executor thread anymore.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Yoann Dubreuil Path: core/src/main/java/hudson/model/queue/WorkUnit.java core/src/main/java/hudson/model/queue/WorkUnitContext.java http://jenkins-ci.org/commit/jenkins/8d3eb20e5d82cf3528849301493ec5b2ddf9dd87 Log: JENKINS-33038 correctly set executor in FutureImpl This patch fixes cancellation of the build linked to the Future object. Before, the executor set in FutureImpl was always empty, because WorkUnitContext.createWorkUnit() is not called from an Executor thread anymore.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Oliver Gondža
          Path:
          core/src/main/java/hudson/model/queue/WorkUnit.java
          core/src/main/java/hudson/model/queue/WorkUnitContext.java
          http://jenkins-ci.org/commit/jenkins/eeea5ee403854ef733036dc9e12bc7d3424608c3
          Log:
          Merge pull request #2043 from ydubreuil/fix-futureimpl

          [FIXED JENKINS-33038] correctly set executor in FutureImpl

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oliver Gondža Path: core/src/main/java/hudson/model/queue/WorkUnit.java core/src/main/java/hudson/model/queue/WorkUnitContext.java http://jenkins-ci.org/commit/jenkins/eeea5ee403854ef733036dc9e12bc7d3424608c3 Log: Merge pull request #2043 from ydubreuil/fix-futureimpl [FIXED JENKINS-33038] correctly set executor in FutureImpl
          Hide
          dogfood dogfood added a comment -

          Integrated in jenkins_main_trunk #4509
          JENKINS-33038 correctly set executor in FutureImpl (Revision 8d3eb20e5d82cf3528849301493ec5b2ddf9dd87)

          Result = SUCCESS
          yoann.dubreuil : 8d3eb20e5d82cf3528849301493ec5b2ddf9dd87
          Files :

          • core/src/main/java/hudson/model/queue/WorkUnit.java
          • core/src/main/java/hudson/model/queue/WorkUnitContext.java
          Show
          dogfood dogfood added a comment - Integrated in jenkins_main_trunk #4509 JENKINS-33038 correctly set executor in FutureImpl (Revision 8d3eb20e5d82cf3528849301493ec5b2ddf9dd87) Result = SUCCESS yoann.dubreuil : 8d3eb20e5d82cf3528849301493ec5b2ddf9dd87 Files : core/src/main/java/hudson/model/queue/WorkUnit.java core/src/main/java/hudson/model/queue/WorkUnitContext.java

            People

            Assignee:
            Unassigned Unassigned
            Reporter:
            ydubreuil Yoann Dubreuil
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: