Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Memory leak through EclipseContext and ActivatePartExpression #178

Closed
lredor opened this issue Nov 28, 2023 · 6 comments
Closed

Memory leak through EclipseContext and ActivatePartExpression #178

lredor opened this issue Nov 28, 2023 · 6 comments
Assignees
@lredor

Comments

@lredor
Copy link
Contributor

lredor commented Nov 28, 2023

After fixing two leaks of DDiagramEditorImpl (#174 and #176), a remaining leak is always here concerning EclipseContext and ActivatePartExpression.

Steps to reproduce (always the same than above issues):

  • Initialize the data
    • Launch a Sirius runtime
    • Switch to Modeling perspective
    • Create a new Modeling Project
    • Replace the content of this project with the 3 files from this folder. This data is used in org.eclipse.sirius.tests.swtbot.layout.ZOrderActionsTest where the leak has been discovered.
    • Close the runtime
  • Launch the Sirius runtime with YourKit (sample of startup options in "YourKit Java Profiler" tab of launch config: exceptions=off,disablestacktelemetry,alloceach=100,allocsizelimit=4096 to have memory analysis enabled)
  • In Model Explorer view, expand the above project, then My.ecore and root
  • Open the diagram diagramWithNodesAndEdges
  • On edge1, launch the action Bring to Front (from contextual menu Format>Order)
  • Then click on the menu Edit>Undo
  • Close the editor without saving
  • Go to YourKit and take a snapshot of the memory
  • Expected: In YourKit, you can observed the following leak concerning DDiagramEditorImpl referenced through the activePart of ActivePartExpression, ... referenced through the activeRATS of EclipseContext.
    YourKit_EclipseContextAndActivatePartExpression
@lredor
Copy link
Contributor Author

lredor commented Nov 28, 2023

I'm not familiar with the code involved, so I will start by describing the observed situation during a debugging session.

The TrackableComputationExt observed in the leak is created during the opening of the editor. Corresponding stacktrace is :

Thread [main] (Suspended (breakpoint at line 31 in org.eclipse.e4.core.internal.contexts.TrackableComputationExt))	
	owns: org.eclipse.swt.widgets.RunnableLock  (id=13725)	
	org.eclipse.e4.core.internal.contexts.TrackableComputationExt.<init>(org.eclipse.e4.core.contexts.RunAndTrack, org.eclipse.e4.core.contexts.IEclipseContext) line: 31	
	org.eclipse.e4.core.internal.contexts.EclipseContext.runAndTrack(org.eclipse.e4.core.contexts.RunAndTrack) line: 338	
	org.eclipse.ui.internal.contexts.ContextService.activateContext(java.lang.String, org.eclipse.core.expressions.Expression) line: 151	
	org.eclipse.ui.internal.contexts.SlaveContextService.doActivateContext(org.eclipse.ui.contexts.IContextActivation) line: 229	
	org.eclipse.ui.internal.contexts.SlaveContextService.activateContext(java.lang.String) line: 121	
	org.eclipse.sirius.diagram.ui.tools.internal.editor.DDiagramEditorImpl(org.eclipse.gmf.runtime.diagram.ui.parts.DiagramEditor).createPartControl(org.eclipse.swt.widgets.Composite) line: 1595	
	org.eclipse.sirius.diagram.ui.tools.internal.editor.DDiagramEditorImpl(org.eclipse.gmf.runtime.diagram.ui.parts.DiagramEditorWithFlyOutPalette).createPartControl(org.eclipse.swt.widgets.Composite) line: 330	
	org.eclipse.sirius.diagram.ui.tools.internal.editor.DDiagramEditorImpl(org.eclipse.gmf.runtime.diagram.ui.resources.editor.parts.DiagramDocumentEditor).createPartControl(org.eclipse.swt.widgets.Composite) line: 1540	
	org.eclipse.sirius.diagram.ui.tools.internal.editor.DDiagramEditorImpl.createPartControl(org.eclipse.swt.widgets.Composite) line: 645	
	org.eclipse.ui.internal.e4.compatibility.CompatibilityEditor(org.eclipse.ui.internal.e4.compatibility.CompatibilityPart).createPartControl(org.eclipse.ui.IWorkbenchPart, org.eclipse.swt.widgets.Composite) line: 158	
	org.eclipse.ui.internal.e4.compatibility.CompatibilityEditor.createPartControl(org.eclipse.ui.IWorkbenchPart, org.eclipse.swt.widgets.Composite) line: 96	
	org.eclipse.ui.internal.e4.compatibility.CompatibilityEditor(org.eclipse.ui.internal.e4.compatibility.CompatibilityPart).create() line: 365	
	...

During the closing of the editor, this TrackableComputationExt is updated several times. I imagine that the "cleaning" must be done during one of this update but I currently don't understand what is supposed to be done.

  • Twice with ContextChangeEvent.ADDED as eventType. Here, the runnable.changed(originatingContext) is called.
Thread [main] (Suspended)	
	org.eclipse.ui.internal.contexts.ContextService$UpdateExpression.changed(org.eclipse.e4.core.contexts.IEclipseContext) line: 124	
	org.eclipse.e4.core.internal.contexts.TrackableComputationExt.update(org.eclipse.e4.core.internal.contexts.ContextChangeEvent) line: 105	
	org.eclipse.e4.core.internal.contexts.EclipseContext.processScheduled(java.util.Set<org.eclipse.e4.core.internal.contexts.EclipseContext.Scheduled>) line: 358	
	org.eclipse.e4.core.internal.contexts.EclipseContext.set(java.lang.String, java.lang.Object) line: 374	
	org.eclipse.e4.core.internal.contexts.EclipseContext.activate() line: 661	
	org.eclipse.e4.core.internal.contexts.EclipseContext.activateBranch() line: 670	
	org.eclipse.e4.ui.internal.workbench.PartActivationHistory.activate(org.eclipse.e4.ui.model.application.ui.basic.MPart, boolean) line: 60	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(org.eclipse.e4.ui.model.application.ui.basic.MPart, boolean, boolean) line: 767	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(org.eclipse.e4.ui.model.application.ui.basic.MPart, boolean) line: 686	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(org.eclipse.e4.ui.model.application.ui.basic.MPart) line: 681	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(org.eclipse.e4.ui.model.application.ui.basic.MPart, boolean) line: 1405	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(org.eclipse.e4.ui.model.application.ui.basic.MPart) line: 1340	
	org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.closePart(org.eclipse.swt.widgets.Widget) line: 1337	
	org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer$3.close(org.eclipse.swt.custom.CTabFolderEvent) line: 1197	
	...
  • A third time with ContextChangeEvent.DISPOSE as eventType. But here, the eventsContext is popup:org.eclipse.gmf.runtime.diagram.ui.DiagramEditorContextMenu and the originatingContext is TrimmedWindowImpl (IDEWindow) Context. So the TrackableComputationExt is not removed form the activeRATs list of EclipseContext (the suspected leak).
Thread [main] (Suspended (breakpoint at line 113 in org.eclipse.e4.core.internal.contexts.TrackableComputationExt))	
	org.eclipse.e4.core.internal.contexts.TrackableComputationExt.update(org.eclipse.e4.core.internal.contexts.ContextChangeEvent) line: 113	
	org.eclipse.e4.core.internal.contexts.TrackableComputationExt.handleInvalid(org.eclipse.e4.core.internal.contexts.ContextChangeEvent, java.util.Set<org.eclipse.e4.core.internal.contexts.EclipseContext.Scheduled>) line: 68	
	org.eclipse.e4.core.internal.contexts.EclipseContext.dispose() line: 179	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.clearContext(org.eclipse.e4.ui.model.application.ui.MContext) line: 986	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.safeRemoveGui(org.eclipse.e4.ui.model.application.ui.MUIElement) line: 959	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run() line: 861	
	org.eclipse.core.runtime.SafeRunner.run(org.eclipse.core.runtime.ISafeRunnable) line: 45	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.removeGui(org.eclipse.e4.ui.model.application.ui.MUIElement) line: 845	
	org.eclipse.e4.ui.workbench.renderers.swt.ContributedPartRenderer.disposeWidget(org.eclipse.e4.ui.model.application.ui.MUIElement) line: 274	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.safeRemoveGui(org.eclipse.e4.ui.model.application.ui.MUIElement) line: 938	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run() line: 861	
	...
	org.eclipse.e4.ui.model.application.ui.basic.impl.PartImpl(org.eclipse.emf.common.notify.impl.BasicNotifierImpl).eNotify(org.eclipse.emf.common.notify.Notification) line: 424	
	org.eclipse.e4.ui.model.application.ui.basic.impl.PartImpl(org.eclipse.e4.ui.model.application.ui.impl.UIElementImpl).setToBeRendered(boolean) line: 314	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(org.eclipse.e4.ui.model.application.ui.basic.MPart, boolean) line: 1410	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(org.eclipse.e4.ui.model.application.ui.basic.MPart) line: 1340	
	org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.closePart(org.eclipse.swt.widgets.Widget) line: 1337	
	org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer$3.close(org.eclipse.swt.custom.CTabFolderEvent) line: 1197	
	...

After, the SlaveContextService, created before the TrackableComputationExt in the initial stack, is disposed. But this does not involve the EclipseContext.

Thread [main] (Suspended (breakpoint at line 197 in org.eclipse.ui.internal.contexts.SlaveContextService))	
	org.eclipse.ui.internal.contexts.SlaveContextService.dispose() line: 197	
	org.eclipse.ui.internal.EditorSite(org.eclipse.ui.internal.PartSite).dispose() line: 343	
	org.eclipse.ui.internal.EditorSite.dispose() line: 141	
	org.eclipse.ui.internal.e4.compatibility.CompatibilityEditor(org.eclipse.ui.internal.e4.compatibility.CompatibilityPart).disposeSite(org.eclipse.ui.internal.PartSite) line: 443	
	org.eclipse.ui.internal.e4.compatibility.CompatibilityEditor.disposeSite(org.eclipse.ui.internal.PartSite) line: 140	
	org.eclipse.ui.internal.e4.compatibility.CompatibilityEditor(org.eclipse.ui.internal.e4.compatibility.CompatibilityPart).internalDisposeSite(org.eclipse.ui.IWorkbenchPartSite) line: 434	
	org.eclipse.ui.internal.e4.compatibility.CompatibilityEditor(org.eclipse.ui.internal.e4.compatibility.CompatibilityPart).invalidate() line: 272	
	org.eclipse.ui.internal.e4.compatibility.CompatibilityEditor(org.eclipse.ui.internal.e4.compatibility.CompatibilityPart).destroy() line: 421	
	jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(java.lang.reflect.Method, java.lang.Object, java.lang.Object[]) line: not available [native method]	
	jdk.internal.reflect.NativeMethodAccessorImpl.invoke(java.lang.Object, java.lang.Object[]) line: 77	
	jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.lang.Object, java.lang.Object[]) line: 43	
	java.lang.reflect.Method.invoke(java.lang.Object, java.lang.Object...) line: 568	
	org.eclipse.e4.core.internal.di.MethodRequestor.execute() line: 58	
	org.eclipse.e4.core.internal.di.InjectorImpl.processAnnotated(java.lang.Class<? extends java.lang.annotation.Annotation>, java.lang.Object, java.lang.Class<?>, org.eclipse.e4.core.di.suppliers.PrimaryObjectSupplier, org.eclipse.e4.core.di.suppliers.PrimaryObjectSupplier, java.util.ArrayList<java.lang.Class<?>>) line: 971	
	org.eclipse.e4.core.internal.di.InjectorImpl.processAnnotated(java.lang.Class<? extends java.lang.annotation.Annotation>, java.lang.Object, java.lang.Class<?>, org.eclipse.e4.core.di.suppliers.PrimaryObjectSupplier, org.eclipse.e4.core.di.suppliers.PrimaryObjectSupplier, java.util.ArrayList<java.lang.Class<?>>) line: 936	
	org.eclipse.e4.core.internal.di.InjectorImpl.uninject(java.lang.Object, org.eclipse.e4.core.di.suppliers.PrimaryObjectSupplier) line: 184	
	org.eclipse.e4.core.internal.di.FieldRequestor(org.eclipse.e4.core.internal.di.Requestor<L>).uninject(java.lang.Object, org.eclipse.e4.core.di.suppliers.PrimaryObjectSupplier) line: 177	
	org.eclipse.e4.core.internal.contexts.ContextObjectSupplier$ContextInjectionListener.update(org.eclipse.e4.core.contexts.IEclipseContext, int, java.lang.Object[]) line: 89	
	org.eclipse.e4.core.internal.contexts.TrackableComputationExt.update(org.eclipse.e4.core.internal.contexts.ContextChangeEvent) line: 103	
	org.eclipse.e4.core.internal.contexts.EclipseContext.removeListenersTo(java.lang.Object) line: 487	
	org.eclipse.e4.core.contexts.ContextInjectionFactory.uninject(java.lang.Object, org.eclipse.e4.core.contexts.IEclipseContext) line: 184	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.safeRemoveGui(org.eclipse.e4.ui.model.application.ui.MUIElement) line: 947	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run() line: 861	
	...
	org.eclipse.e4.ui.model.application.ui.basic.impl.PartImpl(org.eclipse.emf.common.notify.impl.BasicNotifierImpl).eNotify(org.eclipse.emf.common.notify.Notification) line: 424	
	org.eclipse.e4.ui.model.application.ui.basic.impl.PartImpl(org.eclipse.e4.ui.model.application.ui.impl.UIElementImpl).setToBeRendered(boolean) line: 314	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(org.eclipse.e4.ui.model.application.ui.basic.MPart, boolean) line: 1410	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(org.eclipse.e4.ui.model.application.ui.basic.MPart) line: 1340	
	org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.closePart(org.eclipse.swt.widgets.Widget) line: 1337	
	org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer$3.close(org.eclipse.swt.custom.CTabFolderEvent) line: 1197	
	...
  • A fourth time with ContextChangeEvent.UNINJECTED as eventType. In this condition, the TrackableComputationExt is not removed form the activeRATs list of EclipseContext.
Thread [main] (Suspended)	
	org.eclipse.e4.core.internal.contexts.TrackableComputationExt.update(org.eclipse.e4.core.internal.contexts.ContextChangeEvent) line: 112	
	org.eclipse.e4.core.internal.contexts.EclipseContext.removeListenersTo(java.lang.Object) line: 487	
	org.eclipse.e4.core.contexts.ContextInjectionFactory.uninject(java.lang.Object, org.eclipse.e4.core.contexts.IEclipseContext) line: 184	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.safeRemoveGui(org.eclipse.e4.ui.model.application.ui.MUIElement) line: 947	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run() line: 861	
	...	
	org.eclipse.e4.ui.model.application.ui.basic.impl.PartImpl(org.eclipse.emf.common.notify.impl.BasicNotifierImpl).eNotify(org.eclipse.emf.common.notify.Notification) line: 424	
	org.eclipse.e4.ui.model.application.ui.basic.impl.PartImpl(org.eclipse.e4.ui.model.application.ui.impl.UIElementImpl).setToBeRendered(boolean) line: 314	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(org.eclipse.e4.ui.model.application.ui.basic.MPart, boolean) line: 1410	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(org.eclipse.e4.ui.model.application.ui.basic.MPart) line: 1340	
	org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.closePart(org.eclipse.swt.widgets.Widget) line: 1337	
	org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer$3.close(org.eclipse.swt.custom.CTabFolderEvent) line: 1197	
	...
  • A fifth time with ContextChangeEvent.DISPOSE as eventType. But again here, the eventsContext is PartImpl (org.eclipse.e4.ui.compatibility.editor) removeOnHide org.eclipse.sirius.diagram.ui.part.SiriusDiagramEditorIDContext and the originatingContext is TrimmedWindowImpl (IDEWindow) Context. So the TrackableComputationExt is not removed form the activeRATs list of EclipseContext (the suspected leak).
Thread [main] (Suspended (breakpoint at line 113 in org.eclipse.e4.core.internal.contexts.TrackableComputationExt))	
	org.eclipse.e4.core.internal.contexts.TrackableComputationExt.update(org.eclipse.e4.core.internal.contexts.ContextChangeEvent) line: 113	
	org.eclipse.e4.core.internal.contexts.TrackableComputationExt.handleInvalid(org.eclipse.e4.core.internal.contexts.ContextChangeEvent, java.util.Set<org.eclipse.e4.core.internal.contexts.EclipseContext.Scheduled>) line: 68	
	org.eclipse.e4.core.internal.contexts.EclipseContext.dispose() line: 179	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.clearContext(org.eclipse.e4.ui.model.application.ui.MContext) line: 986	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.safeRemoveGui(org.eclipse.e4.ui.model.application.ui.MUIElement) line: 959	
	org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run() line: 861	
	...
	org.eclipse.e4.ui.model.application.ui.basic.impl.PartImpl(org.eclipse.emf.common.notify.impl.BasicNotifierImpl).eNotify(org.eclipse.emf.common.notify.Notification) line: 424	
	org.eclipse.e4.ui.model.application.ui.basic.impl.PartImpl(org.eclipse.e4.ui.model.application.ui.impl.UIElementImpl).setToBeRendered(boolean) line: 314	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(org.eclipse.e4.ui.model.application.ui.basic.MPart, boolean) line: 1410	
	org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(org.eclipse.e4.ui.model.application.ui.basic.MPart) line: 1340	
	org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.closePart(org.eclipse.swt.widgets.Widget) line: 1337	
	org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer$3.close(org.eclipse.swt.custom.CTabFolderEvent) line: 1197	
	...

@lredor
Copy link
Contributor Author

lredor commented Nov 28, 2023

According to the above analysis, do you have any clue about what is wrong?

@lredor
Copy link
Contributor Author

lredor commented Nov 28, 2023

For information, I also opened a discussion on "Eclipse Platform GitHub":eclipse-platform/eclipse.platform#889.

@lredor
Copy link
Contributor Author

lredor commented Nov 29, 2023

As the activation is done here in org.eclipse.gmf.runtime.diagram.ui.parts.DiagramEditor.createPartControl(Composite). I tried to deactivte this context in DiagramEditor.dispose() method, by storing the context activation token at activation, but without success. The leak was always here.

@lredor
Copy link
Contributor Author

lredor commented Nov 29, 2023

A solution has been proposed on the Eclipse Platfrom side. I'm not sure about it. Waiting answer...

lredor added a commit to lredor/eclipse.platform.ui that referenced this issue Nov 29, 2023
@lredor
Set RAT expression to null when deactivating context
805ed0c 
If UpdateExpression updating is set to false, the expression will be no
longer evaluated. So it can be set to null.
This avoids potential memory leaks as for example on Sirius [1]
discussed here [2].

[1] eclipse-sirius/sirius-desktop#178
[2] eclipse-platform/eclipse.platform#889
@lredor lredor mentioned this issue Nov 29, 2023
Merged
laeubi pushed a commit to lredor/eclipse.platform.ui that referenced this issue Dec 16, 2023
@lredor @laeubi
Set RAT expression to null when deactivating context
d6dd5b8 
If UpdateExpression updating is set to false, the expression will be no
longer evaluated. So it can be set to null.
This avoids potential memory leaks as for example on Sirius [1]
discussed here [2].

[1] eclipse-sirius/sirius-desktop#178
[2] eclipse-platform/eclipse.platform#889
laeubi pushed a commit to eclipse-platform/eclipse.platform.ui that referenced this issue Dec 16, 2023
@lredor @laeubi
Set RAT expression to null when deactivating context
2fbae54 
If UpdateExpression updating is set to false, the expression will be no
longer evaluated. So it can be set to null.
This avoids potential memory leaks as for example on Sirius [1]
discussed here [2].

[1] eclipse-sirius/sirius-desktop#178
[2] eclipse-platform/eclipse.platform#889
@lredor lredor self-assigned this Dec 18, 2023
@lredor
Copy link
Contributor Author

lredor commented Dec 18, 2023

The proposed fix has been merged on platform.ui. I've tested with the canary target platform and with this version, the leak is fixed.
image

By "this version", I mean org.eclipse.ui.workbench version 3.131.100.v20231216-1633 or more (version certainly included in Eclipse 2024-03). So as soon as Sirius will used Eclipse 2024-03, this leak will be fixed.

I am therefore closing the issue, even if it is complicated to associate a fix version (milestone) with it.

@lredor lredor closed this as completed Dec 18, 2023
amartya4256 pushed a commit to amartya4256/eclipse.platform.ui that referenced this issue Feb 8, 2024
@lredor @amartya4256
Set RAT expression to null when deactivating context
1d85815 
If UpdateExpression updating is set to false, the expression will be no
longer evaluated. So it can be set to null.
This avoids potential memory leaks as for example on Sirius [1]
discussed here [2].

[1] eclipse-sirius/sirius-desktop#178
[2] eclipse-platform/eclipse.platform#889
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lredor lredor

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@lredor