Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 509759

Summary:	Potential memory leak when closing Sirius session
Product:	[Modeling] Sirius	Reporter:	Pierre Guilet <pierre.guilet>
Component:	Core	Assignee:	Project inbox <sirius.core-inbox>
Status:	CLOSED FIXED	QA Contact:	Julien Dupont <julien.dupont>
Severity:	normal
Priority:	P3	CC:	julien.dupont, maxime.porhel, pierre-charles.david
Version:	4.1.1	Keywords:	triaged
Target Milestone:	5.1.0
Hardware:	All
OS:	All
See Also:	https://git.eclipse.org/r/87800 https://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=e9a585867ce24852f0d9473b614e8cefb5fd987e
Whiteboard:

Description Pierre Guilet

2016-12-29 08:33:41 EST

The method org.eclipse.sirius.ecore.extender.business.internal.permission.AbstractPermissionAuthority.dispose(ResourceSet) called when closing Sirius session fails to clear listeners and locked objects.

Moreover, the permissions registry org.eclipse.sirius.ecore.extender.business.internal.permission.PermissionAuthorityRegistryImpl does not allow to clear an entry in the cash map org.eclipse.sirius.ecore.extender.business.internal.permission.PermissionAuthorityRegistryImpl.resourceSetToAuthority. Indeed it is a weak hash map that will handle the clearing if no strong or soft reference exists on the key.

From the registry, we can only retrieve the IPermissionAuthority associated to a ResourceSet and call the dispose() method on it.

But, if the listeners or locked objects of provided Sirius AbstractPermissionAuthority have some references directly or indirectly to the resource set key it is associated to in the registry, and because currently those are not cleared, then the entry will never be cleared from the cache map of the registry leading to a memory leak.

Comment 1 Pierre Guilet

2016-12-29 08:41:33 EST

Also when closing the Sirius session, we call the method org.eclipse.sirius.business.internal.session.danalysis.Saver.dispose() on the session's Saver object.

But this method does not clear the Saver's transactional editing domain and the resource set inside.
The Saver is also not set to null in the session org.eclipse.sirius.business.internal.session.danalysis.DAnalysisSessionImpl 

So if for any reason the session is kept in memory for example in a provided menu action having such a reference that is not cleared when closing the session, then we have a memory leak.

Comment 2 Eclipse Genie

2016-12-29 08:47:02 EST

New Gerrit change created: https://git.eclipse.org/r/87800

Comment 3 Maxime Porhel

2017-01-19 04:54:26 EST

Hi Pierre, 

Thanks for the detail bug report, analysis and draft patch.

Comment 4 Eclipse Genie

2017-08-22 04:54:44 EDT

Gerrit change https://git.eclipse.org/r/87800 was merged to [master].
Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=e9a585867ce24852f0d9473b614e8cefb5fd987e

Comment 5 Pierre-Charles David

2017-08-22 05:14:47 EDT

Fixed by e9a585867ce24852f0d9473b614e8cefb5fd987e. This is a rather technical issue, which was difficult to reproduce and to observe when it happens (it requires analysing a memory snapshot), but the patch seems safe enough. No tests (manual or otherwise) or validation needed.

Comment 6 Pierre-Charles David

2017-11-08 03:36:48 EST

Available in Sirius 5.1.0, see https://wiki.eclipse.org/Sirius/5.1.0.