| [udig-devel] deadlock +bug + problem description + possible solutions |
Good morning/day!
I was hunting for this deadlock in DEBUG mode for the time being and got it!
LayerAddingInterceptor is a very bad thing. It adds listener, but when the
service is reset the LayerImpl.geoResources member is NULLed, but the stuff
that has been done by different "interceptors" is not released (like feature
listeners being collected in IndexedShapefileDataStore in
FeatureListenerManager).
>From my point of view, listenening changes of features from datastore ..
mm.. is very dangerous thing. It can reduce performance during operations
modifying hundreds of features if for any event lots of code is run.. Where
is a batch notification? or things like that..
Listening can be used for collecting some information, for example, but NOT
for any rendering actions.. Rendering mechanism MUST be as lazy as possible.
Rendering events should come not from heart of DataStore API through feature
listeners..
Whenever some modifications are done at some place only the developer knows
what kind of modifications are performed and what should be repainted. And
at this stage in the end of operation the re-rendering has to be called
once.
Summary:
1) LayerImpl.changed(..) must not call any renering stuff... this leads to
potential deadlock and not optimal performance.
2) Any rendering calls after modifications, transaction committing, rollback
whatever changes DataStore should be called directly, not through listening
of feature changes mechanism that is not optimal and safe. It will maintain
lazy approach also.
Deleting of features, committing, rollback - these operations are just
commands and it is easy to call rendering at the end, if needed of course.
3) Resetting of the service/resource just kills the member of LayerImpl. The
mechanism is needed to do some finalizing actions when cached georesource is
releasing and the new one is created running again all necessary
interceptors.
4) At the figure DefaultMapLayer.gif the problem is obvious.
ShapefileRenderer (if I remember correctly) creates each time the rendering
happens a new DeafaultMapContext objct that leads to adding listener to the
DataStore forever. Each new rendering - new listener.
=================== lyrical digression======================
I would like to see interceptors design more complete by adding lifecycle
management for intercepted resources.
Just a raw outline:
interface IResourceInterceptor{
initialize(.......);
finalize(......);
}
Interceptors are doing something now, but no way to release, remove
listeners whatever.. when cached resources are destroyed somewhere..
For example let's imagine a kind of IMapInterceptor:
public void mapLoaded(IMap map) - map is loaded from XMI.
public void mapOpened(IMap map) - map is opened in MapEditor
public void mapClosed(IMap map) - map is closed and persisted to XMI.
======================================================
I think issues 1),2) are quite important to be considered ASAP.
Vitali.
Attachment:
deadlock.gif
Description: GIF image
Attachment:
DefaultMapLayer.gif
Description: GIF image
