Comment 1 Alex Shatalin 2006-08-28 11:51:57 EDT

Changing component.

Comment 2 Li Ding 2006-12-11 14:01:11 EST

I hit the same problem for my GMF based editor. The TabbedPropertyViewer holds the reference of the editor instance. My editor can not be GCed because of this reference. 

Comment 3 Raj Mandayam 2007-06-15 15:26:13 EDT

It has been almost a year. Is there any progress on this bug.

Comment 4 Li Ding 2007-06-18 17:06:50 EDT

I'd like to increase the severity to critical so it can be fixed sooner:
1. This memory leak has negative effect on performance for GMF based editors because they use tabbed properties view. 
2. Because of it, the editor extends from GMF based editor has memory leak too. 

Comment 5 Anthony Hunter 2007-06-19 11:44:20 EDT

Moving to the next release, GMF 2.1. 

Comment 6 André Lahs 2008-03-19 10:16:46 EDT

I don't know, whether this bug has been fixed yet or not, but if not, I would like to add some hints for solving it. I did it locally:

1. The problem is, that the dispose() method of the XXXSheetLabelProvider is not(!) called on closing the editor. The result of this is, that the AdapterFactoryLabelProvider child of the XXXSheetLabelProvider will not be removed from the adapterFactory of the XXXDiagramEditorPlugin. The AdapterFactoryLabelProvider is the reason for not garbage collecting the editor.
2. When the TabbedPropertyRegistry object (which stores the XXXSheetLabelProvider instance for the editor in the labelProvider property) is disposed by TabbedPropertySheetPage.dispose(), the dispose() Method of XXXSheetLabelProvider must be called. Due to the fact that TabbedPropertySheetPage is not a part of gmf, this could be done in its subclass PropertiesBrowserPage, or it has to be delegated to jface.
3. A concrete solution could be:

public class PropertiesBrowserPage ... {
  ...
  public void dispose() {
    // ADDED
    // createRegistry() seems to return the already created registry for the     
    // contribuor (XXXDiagramEditor)
    TabbedPropertyRegistry registry =
      TabbedPropertyRegistryFactory.getInstance()
        .createRegistry(this.contributor);
		
    registry .getLabelProvider().dispose();	
    // ADDED
    super.dispose();
    ...
  }
  ...
}

I do not know, whether this is the right place to dispose it, but it seemed suitable for me.

Comment 7 André Lahs 2008-03-20 12:22:17 EDT

I have to add a comment on what I wrote above:

As I now understand is, that all editors of one editor class are using the same instance of XXXSheetLabelProvider. For this reason, the XXXSheetLabelProvider cannot be disposed on closing one editor, all editors of one class has to be closed before the XXXSheetLabelProvider can be disposed.

So the best (and only place) to dispose is the disposeRegistry(..) method of the TabbedPropertyRegistryFactory.

public void disposeRegistry(ITabbedPropertySheetPageContributor target) {
  ...
  if (data.references.isEmpty()) {
    //ADDED
    data.registry.labelProvider.dispose();
    //ADDED
    idToCacheData.remove(key);
  }
  ..
}

Are there any problems expected with this solution?

Comment 8 Gary Karasiuk 2008-07-14 14:44:10 EDT

(In reply to comment #3)
> It has been almost a year. Is there any progress on this bug.
> 
Now it's almost been two years? Is this still a problem?

Comment 9 András Péteri 2010-01-07 11:57:41 EST

Created attachment 155515 [details]
Patch to dispose the tabbed property registry's label provider when it's no longer used

It's still a problem. I can confirm André's observations using the GMF mind map example:

1) Start debugging the example RCP application

2) Make sure no diagram editor is open; close the outline view

3) Set breakpoints on MindmapSheetLabelProvider's constructor, addListener(ILabelProviderListener) and removeListener(ILabelProviderListener) methods (the last two are in DecoratingLabelProvider)

4) Open or create a mind map diagram

5) When the first breakpoint is hit, "step into" the code until you reach AdapterFactoryLabelProvider's constructor; note that it registers itself as a listener to MindmapDiagramEditorPlugin's adapterFactory member

6) Resume execution until the second breakpoint is hit; note that the PropertiesBrowserPage registers itself *twice* to the AdapterFactoryLabelProvider via the MindmapSheetLabelProvider instance

7) Resume execution, close the diagram (this causes the associated TabbedPropertyRegistry to be removed from TabbedPropertyRegistryFactory's cache)

8) The third breakpoint is hit; note that the PropertiesBrowserPage removes itself only *once* from the listeners of MindmapSheetLabelProvider; this isn't a problem for its parent class (DecoratingLabelProvider), because it stores listeners in a ListenerList so duplicates aren't stored twice, however the AdapterFactoryLabelProvider uses an ArrayList for storing listeners

9) Resume execution, open or create a second diagram, follow step 5 again

10) Note that previous AdapterFactoryLabelProvider instances remain on adapterFactory's list of listeners; these in turn have one listener reference remaining, pointing to the associated PropertiesBrowserPage, which has a tabListContentProvider attached, which points to the view part that was providing content for the tabbed property view when its registry was disposed. If it happens to be a diagram editor instance, all associated resources are reachable as well through its undoContext member, and they all stay in memory until the plugin is stopped.

Adding two listeners in PropertiesBrowserPage and removing only one might be a bug in itself, but I'm not sure about that.

Attaching a workspace patch based on comment #7.

Comment 10 Eclipse Webmaster 2010-07-19 21:58:16 EDT

[GMF Restructure] Bug 319140 : product GMF and component
Runtime was the original product and component for this bug

Comment 11 Markus Franz 2010-08-26 10:43:44 EDT

This is one really annoying bug. In our (GMF based) application, the resulting memory leak is a huge problem now. I wanted to ask, why this 4 year old bug with a 2 year old proposed solution and a several month old submitted patch still has the status NEW?

Comment 12 Anthony Hunter 2010-09-01 09:15:28 EDT

I will see if we can get this into platform 3.6.1.

Comment 13 Anthony Hunter 2010-09-01 15:20:56 EDT

This issue is a duplicate of bug 275702 which is already fixed in Eclipse 3.7 M1.

We need to see if we can get bug 275702 backported to 3.6.1.

*** This bug has been marked as a duplicate of bug 275702 ***