geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ed Letifov <>
Subject Re: GMBeanEndPoint - ConcurrentModification
Date Tue, 13 Jan 2004 19:27:49 GMT
  since copy on update will not really solve issue of having "stale" 
iterators around I have a sideline question:
  can we consider usage of ConcurrentHashMap or ConcurrentReaderHashMap 
from EDU.oswego.cs.dl.util.concurrent package for this?
  As I understand this code is in public domain and can be used, will 
provide a substantial concurrency level and the iterators will reflect 
the current state of the collection as much as possible.

Ed Letifov.

On Monday, Jan 12, 2004, at 19:57 Europe/Amsterdam, Aaron Mulder wrote:

> 	We may also want to consider that instead of copying the
> Collection at iteration time, we could copy it only at update time, and
> then write the new (changed) version over the old (iterated) version,
> leaving the only references to the old collection with the iterators.
> I'm assuming that we'll have more iterating than updating, and if so, 
> this
> ought to result in less copying.
> Aaron
> On Mon, 12 Jan 2004, Dain Sundstrom wrote:
>> On Jan 12, 2004, at 8:14 AM, gianny DAMOUR wrote:
>>> Hi,
>>> I have just checked the new GBean implementation and I do have the
>>> feeling that there are - potentially - some concurrency issues with
>>> the CollectionProxy:
>> I bet there are.  It was the last one coded and in a rush so we could
>> get the code checked in.  I plan on reviewing all the code again over
>> the next few days, and writing some documentation as I go.
>>> If I understand correctly, this proxy implements its own Collection 
>>> in
>>> order to solve some previous lifecycle limitations. This Collection
>>> returns an Iterator directly backed by an Iterator directly backed by
>>> the values of the endpoint name to proxy Map of CollectionProxy.
>>> Unfortunately, it means that a service can not iterate safely over
>>> such an Iterator without risking a ConcurrentModificationException
>>> under some specific race conditions.
>> We had that problem in the previous code also.  I'm not sure how we
>> want to handle this.
>>> For instance, DeploymentController could iterate over its Collection
>>> of DeploymentPlanner and, at the same time, a DeploymentPlanner could
>>> be unregistered, which should cause a 
>>> ConcurrentModificationException.
>>> One could return an Iterator backed by a copy of the actual Map.
>>> However, this implementation does not work:
>> I have been thinking of that, or we could change the code to
>> synchronized on the client collection whenever making a change to the
>> backing collection, then the client could guarantee a safe iterator by
>> synchronizing on the collection.  The problem with the copy
>> implementation is we could have very old iterators sitting around, and
>> the problem with synchronization is poorly written code locking up the
>> GMBean for a long time.  I'm leaning towards the copy code.
>>> Indeed, this Iterator could returned a disconnected
>>> ProxyMethodInterceptor if at the same time an endpoint is
>>> unregistered. One could use the GeronimoMBeanEndpointListener feature
>>> in order to detect such a case.
>>> However, as endpoints are inter-connected by listening to JMX
>>> notifications, it is also possible to have a connected
>>> ProxyMethodInterceptor, which reflects a JMX invocation against an
>>> already unregistered MBean.
>>> Hence, I would like to know if the following could fix the previous
>>> issues - if there are indeed some concurrency issues:
>>> I propose to add a Mediator, whose responsibility is to track
>>> synchronously (no notifications involved) the availability of
>>> components and contact synchronously the interested components when a
>>> GBean switch from the offline to the online state and conversaly.
>>> Could anyone confirm or disprove my concern and approach?
>> I don't like this approach.  I think we will end up with way too much
>> synchronization in the container.  I'm also not sure it is 
>> possible....
>> This is a highly threaded system, which means that several components
>> could want to change state simultaneously, and if we make the system
>> state changes synchronous, we are bound to get a system wide deadlock.
>> Also the notification happens after the component fails or stops, so
>> even if we hold up the notification to get a clean system wide state
>> change, the component is already unavailable (just not reflected in 
>> the
>> system state).
>> Besides the implementation difficulty, I don't think we want this.  I
>> see the current implementation as a "good enough" design.  Components
>> do die at the wrong time, and all you can do is respond.  This is how 
>> I
>> define a "good enough" design, "It does not solve all the problems, 
>> but
>> it is good enough to work reliably."  If you really needed to work 
>> with
>> a component, you catch the unavailable exception and fail, which is
>> exactly how single valued components work. If you don't really need to
>> work with a specific component, you just catch the exception and move
>> on.  Of course this means that people writing gbeans need to know what
>> they are doing, but I see that as the price of creating reliable
>> production ready services.
>> -dain

View raw message