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

          Jesse Glick created issue -
          Jesse Glick made changes -
          Link New: This issue is related to JENKINS-24380 [ JENKINS-24380 ]

          Jesse Glick added a comment -

          The fix of JENKINS-24380 should have rendered this control flow obsolete.

          Jesse Glick added a comment - The fix of JENKINS-24380 should have rendered this control flow obsolete.
          Jesse Glick made changes -
          Resolution New: Duplicate [ 3 ]
          Status Original: Open [ 1 ] New: Resolved [ 5 ]

          Code changed in jenkins
          User: Jesse Glick
          Path:
          core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java
          core/src/main/java/jenkins/model/lazy/BuildReference.java
          http://jenkins-ci.org/commit/jenkins/1078e6b68fcdce7bf3db459fc0074aca0625bbba
          Log:
          Added assertion as suggested in JENKINS-22767.

          Compare: https://github.com/jenkinsci/jenkins/compare/447fe9c5b8df...1078e6b68fcd

          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/main/java/jenkins/model/lazy/BuildReference.java http://jenkins-ci.org/commit/jenkins/1078e6b68fcdce7bf3db459fc0074aca0625bbba Log: Added assertion as suggested in JENKINS-22767 . Compare: https://github.com/jenkinsci/jenkins/compare/447fe9c5b8df...1078e6b68fcd

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

          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-plugin/7022e83ecadb84717f65682cd3ec72af4c07f13c Log: @RandomlyFails that I suspect is traceable to JENKINS-22767 .
          Jesse Glick made changes -
          Link New: This issue is related to JENKINS-27532 [ JENKINS-27532 ]

          Jesse Glick added a comment -

          The same code path still seems to apply.

          Jesse Glick added a comment - The same code path still seems to apply.
          Jesse Glick made changes -
          Resolution Original: Duplicate [ 3 ]
          Status Original: Resolved [ 5 ] New: Reopened [ 4 ]
          Jesse Glick made changes -
          Status Original: Reopened [ 4 ] New: Open [ 1 ]

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

              Created:
              Updated:
              Resolved: