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

A few comments on Extension Point registration

XMLWordPrintable

    • Icon: Improvement Improvement
    • Resolution: Fixed
    • Icon: Major Major
    • core
    • None
    • Platform: All, OS: All

      I noticed a couple things about the ExensionPoint registration APIs.I just
      though I would mention them here to give some general feedback. I haven't
      experienced any error as a result of these APIs so I'm just adding this comment
      as an enhancement for you to evaluate when you have time,

      • Thread-saftey

      It seems that some lists have COW semantics which would allow the lists to be
      modified directly, while others have no such semantics associated with them.

      For instance, this might cause problems when plugins are start()ed and stopped()
      dynamically since some things like BuildSteps aren't protected, which may create
      an opportunity for ConcurrentModificationExceptions. It may not happen much
      because most plugins seem only to be started at initialization time and left
      running, but if a more dynamic plugin management capability were added this migh
      be problematic.

      Another thing I'm not sure about is how safe is it really to modify these lists?

      For example, if you are pulling Iterator or Iterables out of the COW lists when
      you walk through the collection then that iteration would have an accurate view.
      If you have code that is using size(), get() then you're view of that Collection
      may not be consistent.

        • One place I've found this in the source is AbstractProject.java#490.

      The more users that are actively doing things to Hudson at once, the greater the
      opportunity to see some errors from these things.

      • API consistency,

      There's a couple different ways to register ExtensionPoints, ultimately all add
      something a list. The differences I've seen is how you get to the list and what
      sort of list it is and how you get it. Sometimes there is a getter on a
      singleton that gives you a reference to a List to add directly to, sometimes
      there is a direct reference to a List in a class, and sometimes there is a
      direct reference to a List in an interface. Sometimes the List has some types of
      thread semantics associated with it (e.g. the List is exposed as a COW List)
      sometimes any semantics are hidden and not guaranteed (e.g. this List is a
      generic java.util.List interface)

      While all of these are getting the job done right now, it might be a good idea
      to review these at some point to see if it makes sense to come up with a more
      consistent presentation. Its no a show stopper by any means, but it a little odd
      to plugin writers when the style used to expose registration points seems
      changes somewhat arbitrarily.

      Below is a list of a few places that illustrate the differences:

      COW lists where concurrent modification shouldn't raise an exception:

      Hudson.getJobListeners().add( listener );

      ArrayLists where concurrent modification exceptions may be possible:

      BuildStep.BUILDERS
      BuildWrappers.BUILDERS
      Jobs.PROPERTIES

      Classes with static lists:

      Jobs.PROPERTIES

      Interfaces with static lists:

      BuildStep.BUILDERS

      Classes with getters:

      Hudson.getJobListeners().add( listener );

            Unassigned Unassigned
            crahen crahen
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

              Created:
              Updated:
              Resolved: