Bug 454388 - ComposedAdapterFactory not disposed
Summary: ComposedAdapterFactory not disposed
Status: ASSIGNED
Alias: None
Product: Sirius
Classification: Modeling
Component: Core (show other bugs)
Version: 2.0.0   Edit
Hardware: All All
: P5 normal (vote)
Target Milestone: ---   Edit
Assignee: Laurent Fasani CLA
QA Contact:
URL:
Whiteboard:
Keywords: triaged
Depends on:
Blocks:
 
Reported: 2014-12-08 04:18 EST by Esteban DUGUEPEROUX CLA
Modified: 2020-09-07 12:09 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Esteban DUGUEPEROUX CLA 2014-12-08 04:18:31 EST
In several places in Sirius code, we have ComposedAdapterFactory not disposed and this can lead to memory leak.
This bugzilla is here to analyse each of these places.

Example of places :

- In TransactionalEditingDomain instantiation, a ComposedAdapterFactory is created but not disposed neither set to null on TransactionalEditingDomain.dispose(). If we have no more reference to TransactionalEditingDomain this ComposedAdapterFactory will be garbage collected then it will no more be a problem but in otherwise it is a memory leak.

- Each call to ViewHelper.INSTANCE.createAdapterFactory() create ComposedAdapterFactory not always disposed.
Comment 1 Esteban DUGUEPEROUX CLA 2014-12-08 05:23:52 EST
There is also SiriusEditPlugin$Implementation.adapterFactory() which is disposed only on Eclipse shutdown.
Comment 2 Laurent Fasani CLA 2016-01-21 06:07:41 EST
SiriusEditPlugin$Implementation.adapterFactory is a ComposedAdapterFactory that contains statefull adapter factories

"Stateful adapter factories (like XXXItemProviderAdapterFactory) implement IDisposable interface to let client code release the state when not used anymore. ComposedAdapterFactory also implements this interface and loop over all registered adapter factories implementing IDisposable to dispose them too."

The XXXItemProviderAdapterFactory have, in most cases, a reference to XXXItemProvider that retains the contents of a resource.
In a modeling project, containing a semantic model, just by unfolding the root semantic object, a MYRootItemProvider is created with a reference to the root semantic object and then retains the full content of the resource.

To summurise, once a sirius session is opened, as soon as you start "playing with" the objects of semantic or representation resources are retained(with their content).

As a thought, we could link the lifecycle of all used AdapterFactory for a session to the session itself. These AdapterFactorys could be disposed in oes.b.a.Session.close()
Comment 3 Laurent Fasani CLA 2016-02-12 11:16:25 EST
When instanciated with ComposedAdapterFactory(Descriptor.Registry) constructor, ComposedAdapterFactory knows how to delegate to adapaterFactories that has been declared as extension with the *org.eclipse.emf.edit.itemProviderAdapterFactories* extension point.
All sirius xxxAdapterFactory are declared as extention.
Moreover, for other MM, "Generate Edit code" on genmodel creates the needed extention.

In conclusion, everywhere, in sirius, we create an AdapterFactory we don't need to add explicitly SiriusxxxAdapterFactories. We just have to return *new ComposedAdapterFactory(ComposedAdapterFactory.Descriptor.Registry.INSTANCE)*

ReflectiveItemProviderAdapterFactory is the only adapterFactory that is not declared as extention. 


note that there is currently about 24 call of ComposedAdapterFactory(Collection<? extends AdapterFactory> ) constructor and about 30 call of ComposedAdapterFactory(AdapterFactory adapterFactory) 

A first step could to remove the singleton adapterFactory in xxxUIPlugin so that the object of the session resource are not reained any more.
getItemProvidersAdapterFactory returns a new ComposedAdapterFactory so we have to manage to lifecyle of the returned ComposedAdapterFactory.
Comment 4 Pierre-Charles David CLA 2016-06-08 10:32:25 EDT
Reducing the priority. It'd certainly be nice to close some leaks, but the situation has been the same for years with no catastrophic impact, so it will be on a "best effort" depending on available resources.