-
Improvement
-
Resolution: Fixed
-
Major
-
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 );