[udig-devel] deadlock +bug + problem description + possible solutions

[Jesse Eichar]

Ah yes,  so that hack is finally coming back to bite me :(.  I knew  
it was going to happen but I was putting off fixing that issue  
because there were other important issues.  It is supposed to be the  
task of the renderer to listen for these changes.  Could you make a  
Jira out of this Email?

Jesse


On 26-Oct-06, at 6:22 AM, Vitali Diatchkov wrote:

> 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.
> <deadlock.gif>
> <DefaultMapLayer.gif>
> _______________________________________________
> User-friendly Desktop Internet GIS (uDig)
> http://udig.refractions.net
> http://lists.refractions.net/mailman/listinfo/udig-devel