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

AbstractLazyLoadRunMap.getById subject to race condition with .load

      If Run.onLoad takes a little while to run, or generally if RunMap.retrieve is not instantaneous, a race condition can arise. Say thread 1 calls getByNumber for a Run which in fact exists on disk (with a numeric symlink). search will call load(int, null), which will delegate to load(File, null) which acquires the lock; and then blocks for a while in retrieve. Then thread 2 calls getById with the ID of the same build. index.byId is still empty, so it jumps to load(String, null) and then blocks waiting to enter load(File, null). Now thread 1 finishes retrieve, stores the newly loaded Run in the index, and returns. When the lock is released, thread 2 enters it, loads another copy of the same Run, and stores it in the index, clobbering the first entry, and returns that.

      Seems to be fixable by making getById double-check the index while holding the lock, forcing it to wait for the first thread to store an entry. (Or could just check under lock whether snapshot != index, and if so, retry.)

      Also load(File, null) should assert that there is no current entry for a build before it stores one.

          [JENKINS-22767] AbstractLazyLoadRunMap.getById subject to race condition with .load

          Code changed in jenkins
          User: Jesse Glick
          Path:
          core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java
          core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
          http://jenkins-ci.org/commit/jenkins/005ffa54fa03990933daaf73c133dcbfbd494b78
          Log:
          Merge pull request #1989 from jglick/AbstractLazyLoadRunMap-JENKINS-22767

          JENKINS-22767 Make sure only one thread actually loads a given build

          Compare: https://github.com/jenkinsci/jenkins/compare/65d92ea982cf...005ffa54fa03

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java http://jenkins-ci.org/commit/jenkins/005ffa54fa03990933daaf73c133dcbfbd494b78 Log: Merge pull request #1989 from jglick/AbstractLazyLoadRunMap- JENKINS-22767 JENKINS-22767 Make sure only one thread actually loads a given build Compare: https://github.com/jenkinsci/jenkins/compare/65d92ea982cf...005ffa54fa03

          Code changed in jenkins
          User: Jesse Glick
          Path:
          changelog.html
          http://jenkins-ci.org/commit/jenkins/0f50ca826470d55ada4a982548d03879dd584e66
          Log:
          JENKINS-22767 Noting merge of #1989.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: changelog.html http://jenkins-ci.org/commit/jenkins/0f50ca826470d55ada4a982548d03879dd584e66 Log: JENKINS-22767 Noting merge of #1989.

          dogfood added a comment -

          Integrated in jenkins_main_trunk #4423
          [FIXED JENKINS-22767] Make sure only one thread actually loads a given (Revision d5167025a204750633c931ea8c1fff8d7561ab9c)

          Result = SUCCESS
          jesse glick : d5167025a204750633c931ea8c1fff8d7561ab9c
          Files :

          • core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
          • core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java

          dogfood added a comment - Integrated in jenkins_main_trunk #4423 [FIXED JENKINS-22767] Make sure only one thread actually loads a given (Revision d5167025a204750633c931ea8c1fff8d7561ab9c) Result = SUCCESS jesse glick : d5167025a204750633c931ea8c1fff8d7561ab9c Files : core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java

          dogfood added a comment -

          Integrated in jenkins_main_trunk #4424
          JENKINS-22767 Noting merge of #1989. (Revision 0f50ca826470d55ada4a982548d03879dd584e66)

          Result = UNSTABLE
          jesse glick : 0f50ca826470d55ada4a982548d03879dd584e66
          Files :

          • changelog.html

          dogfood added a comment - Integrated in jenkins_main_trunk #4424 JENKINS-22767 Noting merge of #1989. (Revision 0f50ca826470d55ada4a982548d03879dd584e66) Result = UNSTABLE jesse glick : 0f50ca826470d55ada4a982548d03879dd584e66 Files : changelog.html

          Code changed in jenkins
          User: Jesse Glick
          Path:
          job/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java
          http://jenkins-ci.org/commit/workflow-plugin/a8073443b901b0cde63dacec2f82f9691833744b
          Log:
          JENKINS-22767 Noting symptom.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: job/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java http://jenkins-ci.org/commit/workflow-plugin/a8073443b901b0cde63dacec2f82f9691833744b Log: JENKINS-22767 Noting symptom.

          Code changed in jenkins
          User: Jesse Glick
          Path:
          core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java
          core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
          http://jenkins-ci.org/commit/jenkins/383a5aec51631412a10c4c8826448af057e606a6
          Log:
          [FIXED JENKINS-22767] Make sure only one thread actually loads a given build.
          (cherry picked from commit d5167025a204750633c931ea8c1fff8d7561ab9c)

          Compare: https://github.com/jenkinsci/jenkins/compare/54c4b7fe32c4...383a5aec5163

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java http://jenkins-ci.org/commit/jenkins/383a5aec51631412a10c4c8826448af057e606a6 Log: [FIXED JENKINS-22767] Make sure only one thread actually loads a given build. (cherry picked from commit d5167025a204750633c931ea8c1fff8d7561ab9c) Compare: https://github.com/jenkinsci/jenkins/compare/54c4b7fe32c4...383a5aec5163

          jglick, this seems to caused JENKINS-33219.

          Oliver Gondža added a comment - jglick , this seems to caused JENKINS-33219 .

          Re-reading the description, the problem appears after the downgrade as well. So I guess it is unrelated.

          Oliver Gondža added a comment - Re-reading the description, the problem appears after the downgrade as well. So I guess it is unrelated.

          Code changed in jenkins
          User: Jesse Glick
          Path:
          aggregator/src/test/java/org/jenkinsci/plugins/workflow/WorkflowTest.java
          http://jenkins-ci.org/commit/workflow-cps-plugin/4f2a44ba03990b8babd3b5387f17f7ed33eef6dc
          Log:
          @RandomlyFails that I suspect is traceable to JENKINS-22767.
          Originally-Committed-As: 7022e83ecadb84717f65682cd3ec72af4c07f13c

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: aggregator/src/test/java/org/jenkinsci/plugins/workflow/WorkflowTest.java http://jenkins-ci.org/commit/workflow-cps-plugin/4f2a44ba03990b8babd3b5387f17f7ed33eef6dc Log: @RandomlyFails that I suspect is traceable to JENKINS-22767 . Originally-Committed-As: 7022e83ecadb84717f65682cd3ec72af4c07f13c

          Code changed in jenkins
          User: Jesse Glick
          Path:
          job/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java
          http://jenkins-ci.org/commit/workflow-job-plugin/c9849559467a7e3af97953262f4bd274031b523b
          Log:
          JENKINS-22767 Noting symptom.
          Originally-Committed-As: a8073443b901b0cde63dacec2f82f9691833744b

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: job/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java http://jenkins-ci.org/commit/workflow-job-plugin/c9849559467a7e3af97953262f4bd274031b523b Log: JENKINS-22767 Noting symptom. Originally-Committed-As: a8073443b901b0cde63dacec2f82f9691833744b

            jglick Jesse Glick
            jglick Jesse Glick
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: