Bug 351334

Comment 1 Sebastian Zarnekow 2011-08-16 17:32:52 EDT

Scheduled for 2.1

Comment 2 Lieven Lemiengre 2011-08-23 05:07:23 EDT

Created attachment 201975 [details]
Root analysis of one of the leaked objects

Comment 3 Lieven Lemiengre 2011-08-23 05:09:13 EDT

Created attachment 201976 [details]
Difference between first build and repeating step 3 four times

It seems we underestimated the impact of this bug.
The memory leak gets progressively worse.

Steps to reproduce:
1) Open a project with big (~10000 loc) files.
2) Perform a clean. (check the amount of used memory)
3) Open/close multiple files in the editor and perform a clean again, do this several times.
4) Close all editors.
You should notice a substantial increase in memory usage

In my case, in this test project I go from 90MB memory usage to 130MB memory usage. If I keep repeating step 3 I can make the leak progressively bigger until I eventually run out of heapspace.
The leaked memory is held by the listeners of in the ChainedPreferenceStore, specifically the OutlineWithEditorLinker listeners. Most of the leaked memory is made out of Strings and TokenInfos.

I've attached a number of screenshots from yourkit to help your analysis.

I think the bug importance should be bumped up to 'major'.

Comment 4 Lieven Lemiengre 2011-08-23 05:10:02 EDT

Created attachment 201978 [details]
The leaked objects and their size

Comment 5 Jan Koehnlein 2011-08-30 11:09:06 EDT

Now nulling the reference on dispose(). Does that fix your issue?

Comment 6 Lieven Lemiengre 2011-09-08 10:21:33 EDT

Created attachment 202995 [details]
patch

The issue was almost fixed, treeViewer and textViewer should be made null aswell.

Comment 7 Jan Koehnlein 2011-09-12 09:15:54 EDT

Thanks. I pushed the fix to MASTER.

Comment 8 Samantha Chan 2011-09-20 16:15:42 EDT

Is the fix of this problem only involves nulling out OutlinePage, textViewer and treeViewer?

I tried to implement this fix in my own code since I cannot update to the latest Xtext but need to address the memory leak.  However, I got the following NPE in this scenario:
1)  open the two editors
2)  close one of the editor
3)  switch to the remaining editor
4)  click on link with editor action.  

Got NPE at OutlineWithEditorLinker:100 @ OutlineWithEditorLinker.setLinkingEnabled(...)

I am wondering if I am missing something or if there is another fix that I need to pick up.

Please note that in my case, the OutlineWithEditorLinker.register does not get called when I switch editor.  i.e. the treeViewer is never initialized after we null out the treeViewer when closing an editor.

Comment 9 Samantha Chan 2011-09-20 17:01:05 EDT

Looking at this a bit more, looks like it's leaking OutlineWithEditorLinker as preference store's property listener.  Even after the linker has been de-registered, it is still being notified about property change and resulted in NPE.

Are you seeing this same problem?

Comment 10 Sven Efftinge 2011-09-21 02:52:31 EDT

We used 

getPreferenceStoreAccess().getPreferenceStore().addPropertyChangeListener

and

getPreferenceStoreAccess().getPreferenceStore().removePropertyChangeListener

Unfortunately getPreferenceStoreAccess().getPreferenceStore() returns a new instance of ChainedPreferenceStore each time it is called. So that was a memory leak.
I fixed it in Xtext 2.1 sream, i.e. master.
For earlier version you need to override LinkWithEditorOutlineContribution with the following:

        private IPreferenceStore preferenceStore;

	@Override
	public void register(OutlinePage outlinePage) {
		IToolBarManager toolBarManager = outlinePage.getSite().getActionBars().getToolBarManager();
		toolBarManager.add(getAction());
		addPropertyChangeListener();
		OutlineWithEditorLinker outlineWithEditorLinker = outlineWithEditorLinkerProvider.get();
		outlineWithEditorLinker.activate(outlinePage);
		outlineWithEditorLinker.setLinkingEnabled(isPropertySet());
		preferenceStore = getPreferenceStoreAccess().getPreferenceStore();
		preferenceStore.addPropertyChangeListener(outlineWithEditorLinker);
		page2linker.put(outlinePage, outlineWithEditorLinker);
	}

	@Override
	public void deregister(OutlinePage outlinePage) {
		removePropertyChangeListener();
		OutlineWithEditorLinker outlineWithEditorLinker = page2linker.remove(outlinePage);
		if (outlineWithEditorLinker != null) {
			outlineWithEditorLinker.deactivate();
			preferenceStore.removePropertyChangeListener(outlineWithEditorLinker);
		}
	}

Comment 11 Karsten Thoms 2017-09-19 17:38:04 EDT

Closing all bugs that were set to RESOLVED before Neon.0

Comment 12 Karsten Thoms 2017-09-19 17:49:15 EDT

Closing all bugs that were set to RESOLVED before Neon.0