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
