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

AbstractLazyLoadRunMap.getById subject to race condition with .load

    Details

    • Similar Issues:

      Description

      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.

        Attachments

          Issue Links

            Activity

            jglick Jesse Glick created issue -
            jglick Jesse Glick made changes -
            Field Original Value New Value
            Link This issue is related to JENKINS-24380 [ JENKINS-24380 ]
            Hide
            jglick Jesse Glick added a comment -

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

            Show
            jglick Jesse Glick added a comment - The fix of JENKINS-24380 should have rendered this control flow obsolete.
            jglick Jesse Glick made changes -
            Status Open [ 1 ] Resolved [ 5 ]
            Resolution Duplicate [ 3 ]
            Hide
            scm_issue_link 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

            Show
            scm_issue_link 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
            Hide
            scm_issue_link 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.

            Show
            scm_issue_link 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 .
            jglick Jesse Glick made changes -
            Link This issue is related to JENKINS-27532 [ JENKINS-27532 ]
            Hide
            jglick Jesse Glick added a comment -

            The same code path still seems to apply.

            Show
            jglick Jesse Glick added a comment - The same code path still seems to apply.
            jglick Jesse Glick made changes -
            Resolution Duplicate [ 3 ]
            Status Resolved [ 5 ] Reopened [ 4 ]
            jglick Jesse Glick made changes -
            Status Reopened [ 4 ] Open [ 1 ]
            jglick Jesse Glick made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            Hide
            jglick Jesse Glick added a comment -

            This workflow-plugin commit works around the bug. Reproduced in a unit test.

            Show
            jglick Jesse Glick added a comment - This workflow-plugin commit works around the bug. Reproduced in a unit test.
            jglick Jesse Glick made changes -
            Remote Link This issue links to "PR 1989 (Web Link)" [ 13757 ]
            jglick Jesse Glick made changes -
            Labels lazy-loading threads lazy-loading lts-candidate threads
            Hide
            scm_issue_link 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/d5167025a204750633c931ea8c1fff8d7561ab9c
            Log:
            [FIXED JENKINS-22767] Make sure only one thread actually loads a given build.

            Show
            scm_issue_link 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/d5167025a204750633c931ea8c1fff8d7561ab9c Log: [FIXED JENKINS-22767] Make sure only one thread actually loads a given build.
            scm_issue_link SCM/JIRA link daemon made changes -
            Status In Progress [ 3 ] Resolved [ 5 ]
            Resolution Fixed [ 1 ]
            Hide
            scm_issue_link 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

            Show
            scm_issue_link 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
            Hide
            scm_issue_link 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.

            Show
            scm_issue_link 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.
            Hide
            dogfood 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
            Show
            dogfood 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
            Hide
            dogfood 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
            Show
            dogfood 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
            Hide
            scm_issue_link 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.

            Show
            scm_issue_link 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.
            jglick Jesse Glick made changes -
            Link This issue is related to JENKINS-26761 [ JENKINS-26761 ]
            jglick Jesse Glick made changes -
            Link This issue is duplicated by JENKINS-32755 [ JENKINS-32755 ]
            olivergondza Oliver Gondža made changes -
            Labels lazy-loading lts-candidate threads 1.642.3-fixed lazy-loading threads
            Hide
            scm_issue_link 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

            Show
            scm_issue_link 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
            Hide
            olivergondza Oliver Gondža added a comment -

            Jesse Glick, this seems to caused JENKINS-33219.

            Show
            olivergondza Oliver Gondža added a comment - Jesse Glick , this seems to caused JENKINS-33219 .
            olivergondza Oliver Gondža made changes -
            Link This issue depends on JENKINS-33219 [ JENKINS-33219 ]
            Hide
            olivergondza Oliver Gondža added a comment -

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

            Show
            olivergondza Oliver Gondža added a comment - Re-reading the description, the problem appears after the downgrade as well. So I guess it is unrelated.
            Hide
            scm_issue_link 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

            Show
            scm_issue_link 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
            Hide
            scm_issue_link 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

            Show
            scm_issue_link 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
            rtyler R. Tyler Croy made changes -
            Workflow JNJira [ 154881 ] JNJira + In-Review [ 195067 ]
            jglick Jesse Glick made changes -
            Link This issue relates to JENKINS-37121 [ JENKINS-37121 ]
            jglick Jesse Glick made changes -
            Link This issue is duplicated by JENKINS-38509 [ JENKINS-38509 ]

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: