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

Bug 501263

Summary:	All properties must be disposed into DTreeEditor.dispose()
Product:	[Modeling] Sirius	Reporter:	Yann Mortier <yann.mortier>
Component:	Tree	Assignee:	Pierre-Charles David <pierre-charles.david>
Status:	CLOSED FIXED	QA Contact:	Pierre Guilet <pierre.guilet>
Severity:	normal
Priority:	P3	CC:	maxime.porhel, pierre-charles.david
Version:	3.1.0	Keywords:	triaged
Target Milestone:	4.1.1
Hardware:	PC
OS:	Windows 7
See Also:	https://git.eclipse.org/r/80929 https://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=d7b5834511b4ba3ce259f47daf0bcf2e0a1e304b https://git.eclipse.org/r/83685 https://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=bb18387550e27d3e0f5d41c94d35fc03730b517c https://git.eclipse.org/r/85740 https://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=b4b937dac3190f416cd482eaf8ca5993a4d56b36
Whiteboard:
Bug Depends on:
Bug Blocks:	506082, 506083

Description Yann Mortier

2016-09-12 10:59:01 EDT

Version: 3.1.3

DTreeEditor does not dispose all its properties:
 * treeModel
 * AbstractDTreeEditor.session
 * AbstractDTreeEditor.undoRedoActionHandler
 * adapterFactory is disposed into DTreeEditor instead AbstractDTreeEditor
 * etc.

All editors should be reviewed in order to check that all properties are properly disposed and/or set to null.

Comment 1 Pierre-Charles David

2016-09-12 11:34:54 EDT

I have not actually measured the implied leak, but from looking at the code it is indeed present, and still here on master. The fix seems obvious at first glance, but I'd like to see the leak with my own eyes to make sure we're not missing something.

From a private discussion with Yann, it seems that one piece of code which keeps a reference to the disposed editors is in org.eclipse.ui.internal.contexts.ContextService.UpdateExpression#expression, which can be an ActivePartExpression with its own activePart field referencing the DTreeEditor.

Comment 2 Eclipse Genie

2016-09-12 11:38:42 EDT

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

Comment 3 Eclipse Genie

2016-10-18 11:26:57 EDT

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

Comment 4 Pierre-Charles David

2016-10-18 11:29:55 EDT

Should be fixed. It's probably not possible to make an automated test for this, but we'll wait for our complete test suites to run to see if I introduced any regression, and some YourKit session is probably possible to confirm the leak is gone.

Comment 5 Eclipse Genie

2016-10-21 09:25:39 EDT

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

Comment 6 Eclipse Genie

2016-10-21 11:06:45 EDT

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

Comment 7 Pierre-Charles David

2016-10-21 11:09:13 EDT

Fixed with bb18387550e27d3e0f5d41c94d35fc03730b517c.

Comment 8 Pierre-Charles David

2016-10-26 04:28:54 EDT

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

Comment 9 Eclipse Genie

2016-11-25 03:48:55 EST

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

Comment 10 Eclipse Genie

2016-12-13 04:00:44 EST

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