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

DescriptorExtensionList not locking correctly, leading to deadlocks

    XMLWordPrintable

Details

    • Bug
    • Status: Resolved (View Workflow)
    • Critical
    • Resolution: Fixed
    • core
    • None
    • Jenkins 2.163

    Description

      From my analysis, and some of other issues reported speaking about deadlocks (JENKINS-20988, JENKINS-21034, JENKINS-31622, JENKINS-44564, JENKINS-49038, JENKINS-50663, JENKINS-54974), the issue lies in the DescriptorExtensionList, especially in the way it acquires its load lock.
      The DescriptorExtensionList getLoadLock method documentation indicates that it is taking part in the real load activity, and that as such, it can lock on this rather than on the singleton Lock used by ExtensionList.

      However, many plugins rely on a GlobalConfiguration object, which is acquired through a code similar to the following (which is actually explicitly recommended in GlobalConfiguration documentation).

      public static SpecificPluginConfig get() {
          return GlobalConfiguration.all().get(SpecificPluginConfig.class);
      }
      

      (the all() method from the GlobalConfiguration is returning a DescriptorExtensionList)

      As the configuration for a plugin can be called from many places (initialization of plugin, http requests, ...), it is very easy to have at the same time a DescriptorExtensionList being instantiated, needing in return an ExtensionList, while at the same time, some injection code will have taken the ExtensionList lock and will require DescriptorExtensionList one.
      Of course, some other uses of DescriptorExtensionList, not related to GlobalConfiguration, can also create the same kind of issues.

      Taking in to account that the lock is only taken when the list is initialized for the first time (in ensureLoaded()), I would say that removing the override of getLoadLock in DescriptorExtensionList should solve the issue at a very minimal cost, or at least make the lock the same as ExtensionList for Descriptor.class

      Attachments

        Issue Links

          Activity

            greybird Arnaud TAMAILLON created issue -
            greybird Arnaud TAMAILLON made changes -
            Field Original Value New Value
            Description From my analysis, and some of other issues reported speaking about deadlocks (JENKINS-20988, JENKINS-21034, JENKINS-31622, JENKINS-44564, JENKINS-49038, JENKINS-50663), the issue lies in the [DescriptorExtensionList|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java], especially in the way it acquires its load lock.
             The DescriptorExtensionList [getLoadLock|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java#L174] method documentation indicates that it is taking part in the real load activity, and that as such, it can lock on *this *rather than on the *singleton Lock *used by ExtensionList.

            However, many plugins rely on a GlobalConfiguration object, which is acquired through a code similar to the following (which is actually explicitly recommended in [GlobalConfiguration documentation|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/jenkins/model/GlobalConfiguration.java#L28]).
            {code:java}
            public static SpecificPluginConfig get() {
                return GlobalConfiguration.all().get(SpecificPluginConfig.class);
            }
            {code}
            (the *all()* method from the GlobalConfiguration is returning a *DescriptorExtensionList*)

            As the configuration for a plugin can be called from many places (initialization of plugin, http requests, ...), it is very easy to have at the same time a DescriptorExtensionList being instantiated, needing in return an ExtensionList, while at the same time, some injection code will have taken the ExtensionList lock and will require DescriptorExtensionList one.
             Of course, some other uses of DescriptorExtensionList, not related to GlobalConfiguration, can also create the same kind of issues.

            Taking in to account that the lock is only taken when the list is initialized for the first time (in [ensureLoaded()|https://github.com/jenkinsci/jenkins/blob/22aa2e6e766074d11249893e3f35e0b99e20d3d0/core/src/main/java/hudson/ExtensionList.java#L316]), I would say that removing the override of [getLoadLock in DescriptorExtensionList|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java#L178] should solve the issue at a very minimal cost, or at least make the lock the same as ExtensionList for Descriptor.class
            greybird Arnaud TAMAILLON made changes -
            Link This issue is related to JENKINS-20988 [ JENKINS-20988 ]
            greybird Arnaud TAMAILLON made changes -
            Link This issue is related to JENKINS-21034 [ JENKINS-21034 ]
            greybird Arnaud TAMAILLON made changes -
            Link This issue is related to JENKINS-31622 [ JENKINS-31622 ]
            greybird Arnaud TAMAILLON made changes -
            Link This issue is related to JENKINS-44564 [ JENKINS-44564 ]
            greybird Arnaud TAMAILLON made changes -
            Link This issue is related to JENKINS-49038 [ JENKINS-49038 ]
            greybird Arnaud TAMAILLON made changes -
            Link This issue is related to JENKINS-50663 [ JENKINS-50663 ]
            greybird Arnaud TAMAILLON made changes -
            Link This issue is related to JENKINS-54974 [ JENKINS-54974 ]
            greybird Arnaud TAMAILLON made changes -
            Description From my analysis, and some of other issues reported speaking about deadlocks (JENKINS-20988, JENKINS-21034, JENKINS-31622, JENKINS-44564, JENKINS-49038, JENKINS-50663), the issue lies in the [DescriptorExtensionList|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java], especially in the way it acquires its load lock.
             The DescriptorExtensionList [getLoadLock|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java#L174] method documentation indicates that it is taking part in the real load activity, and that as such, it can lock on *this *rather than on the *singleton Lock *used by ExtensionList.

            However, many plugins rely on a GlobalConfiguration object, which is acquired through a code similar to the following (which is actually explicitly recommended in [GlobalConfiguration documentation|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/jenkins/model/GlobalConfiguration.java#L28]).
            {code:java}
            public static SpecificPluginConfig get() {
                return GlobalConfiguration.all().get(SpecificPluginConfig.class);
            }
            {code}
            (the *all()* method from the GlobalConfiguration is returning a *DescriptorExtensionList*)

            As the configuration for a plugin can be called from many places (initialization of plugin, http requests, ...), it is very easy to have at the same time a DescriptorExtensionList being instantiated, needing in return an ExtensionList, while at the same time, some injection code will have taken the ExtensionList lock and will require DescriptorExtensionList one.
             Of course, some other uses of DescriptorExtensionList, not related to GlobalConfiguration, can also create the same kind of issues.

            Taking in to account that the lock is only taken when the list is initialized for the first time (in [ensureLoaded()|https://github.com/jenkinsci/jenkins/blob/22aa2e6e766074d11249893e3f35e0b99e20d3d0/core/src/main/java/hudson/ExtensionList.java#L316]), I would say that removing the override of [getLoadLock in DescriptorExtensionList|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java#L178] should solve the issue at a very minimal cost, or at least make the lock the same as ExtensionList for Descriptor.class
            From my analysis, and some of other issues reported speaking about deadlocks (JENKINS-20988, JENKINS-21034, JENKINS-31622, JENKINS-44564, JENKINS-49038, JENKINS-50663, JENKINS-54974), the issue lies in the [DescriptorExtensionList|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java], especially in the way it acquires its load lock.
             The DescriptorExtensionList [getLoadLock|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java#L174] method documentation indicates that it is taking part in the real load activity, and that as such, it can lock on *this *rather than on the *singleton Lock *used by ExtensionList.

            However, many plugins rely on a GlobalConfiguration object, which is acquired through a code similar to the following (which is actually explicitly recommended in [GlobalConfiguration documentation|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/jenkins/model/GlobalConfiguration.java#L28]).
            {code:java}
            public static SpecificPluginConfig get() {
                return GlobalConfiguration.all().get(SpecificPluginConfig.class);
            }
            {code}
            (the *all()* method from the GlobalConfiguration is returning a *DescriptorExtensionList*)

            As the configuration for a plugin can be called from many places (initialization of plugin, http requests, ...), it is very easy to have at the same time a DescriptorExtensionList being instantiated, needing in return an ExtensionList, while at the same time, some injection code will have taken the ExtensionList lock and will require DescriptorExtensionList one.
             Of course, some other uses of DescriptorExtensionList, not related to GlobalConfiguration, can also create the same kind of issues.

            Taking in to account that the lock is only taken when the list is initialized for the first time (in [ensureLoaded()|https://github.com/jenkinsci/jenkins/blob/22aa2e6e766074d11249893e3f35e0b99e20d3d0/core/src/main/java/hudson/ExtensionList.java#L316]), I would say that removing the override of [getLoadLock in DescriptorExtensionList|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java#L178] should solve the issue at a very minimal cost, or at least make the lock the same as ExtensionList for Descriptor.class
            jglick Jesse Glick made changes -
            Assignee Arnaud TAMAILLON [ greybird ]
            jglick Jesse Glick made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            jglick Jesse Glick made changes -
            Status In Progress [ 3 ] In Review [ 10005 ]
            jglick Jesse Glick made changes -
            Remote Link This issue links to "PR 3828 (Web Link)" [ 22152 ]
            batmat Baptiste Mathus made changes -
            Description From my analysis, and some of other issues reported speaking about deadlocks (JENKINS-20988, JENKINS-21034, JENKINS-31622, JENKINS-44564, JENKINS-49038, JENKINS-50663, JENKINS-54974), the issue lies in the [DescriptorExtensionList|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java], especially in the way it acquires its load lock.
             The DescriptorExtensionList [getLoadLock|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java#L174] method documentation indicates that it is taking part in the real load activity, and that as such, it can lock on *this *rather than on the *singleton Lock *used by ExtensionList.

            However, many plugins rely on a GlobalConfiguration object, which is acquired through a code similar to the following (which is actually explicitly recommended in [GlobalConfiguration documentation|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/jenkins/model/GlobalConfiguration.java#L28]).
            {code:java}
            public static SpecificPluginConfig get() {
                return GlobalConfiguration.all().get(SpecificPluginConfig.class);
            }
            {code}
            (the *all()* method from the GlobalConfiguration is returning a *DescriptorExtensionList*)

            As the configuration for a plugin can be called from many places (initialization of plugin, http requests, ...), it is very easy to have at the same time a DescriptorExtensionList being instantiated, needing in return an ExtensionList, while at the same time, some injection code will have taken the ExtensionList lock and will require DescriptorExtensionList one.
             Of course, some other uses of DescriptorExtensionList, not related to GlobalConfiguration, can also create the same kind of issues.

            Taking in to account that the lock is only taken when the list is initialized for the first time (in [ensureLoaded()|https://github.com/jenkinsci/jenkins/blob/22aa2e6e766074d11249893e3f35e0b99e20d3d0/core/src/main/java/hudson/ExtensionList.java#L316]), I would say that removing the override of [getLoadLock in DescriptorExtensionList|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java#L178] should solve the issue at a very minimal cost, or at least make the lock the same as ExtensionList for Descriptor.class
            From my analysis, and some of other issues reported speaking about deadlocks (JENKINS-20988, JENKINS-21034, JENKINS-31622, JENKINS-44564, JENKINS-49038, JENKINS-50663, JENKINS-54974), the issue lies in the [DescriptorExtensionList|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java], especially in the way it acquires its load lock.
             The DescriptorExtensionList [getLoadLock|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java#L174] method documentation indicates that it is taking part in the real load activity, and that as such, it can lock on *this* rather than on the *singleton Lock* used by ExtensionList.

            However, many plugins rely on a GlobalConfiguration object, which is acquired through a code similar to the following (which is actually explicitly recommended in [GlobalConfiguration documentation|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/jenkins/model/GlobalConfiguration.java#L28]).
            {code:java}
            public static SpecificPluginConfig get() {
                return GlobalConfiguration.all().get(SpecificPluginConfig.class);
            }
            {code}
            (the *all()* method from the GlobalConfiguration is returning a *DescriptorExtensionList*)

            As the configuration for a plugin can be called from many places (initialization of plugin, http requests, ...), it is very easy to have at the same time a DescriptorExtensionList being instantiated, needing in return an ExtensionList, while at the same time, some injection code will have taken the ExtensionList lock and will require DescriptorExtensionList one.
             Of course, some other uses of DescriptorExtensionList, not related to GlobalConfiguration, can also create the same kind of issues.

            Taking in to account that the lock is only taken when the list is initialized for the first time (in [ensureLoaded()|https://github.com/jenkinsci/jenkins/blob/22aa2e6e766074d11249893e3f35e0b99e20d3d0/core/src/main/java/hudson/ExtensionList.java#L316]), I would say that removing the override of [getLoadLock in DescriptorExtensionList|https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/DescriptorExtensionList.java#L178] should solve the issue at a very minimal cost, or at least make the lock the same as ExtensionList for Descriptor.class
            oleg_nenashev Oleg Nenashev made changes -
            Released As Jenkins 2.163
            Resolution Fixed [ 1 ]
            Status In Review [ 10005 ] Resolved [ 5 ]
            oleg_nenashev Oleg Nenashev made changes -
            Priority Minor [ 4 ] Critical [ 2 ]
            oleg_nenashev Oleg Nenashev made changes -
            Labels lts-candidate
            olivergondza Oliver Gond┼ża made changes -
            Labels lts-candidate
            jglick Jesse Glick made changes -
            Link This issue relates to JENKINS-53998 [ JENKINS-53998 ]

            People

              greybird Arnaud TAMAILLON
              greybird Arnaud TAMAILLON
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: