Bug 397061

Description Chris Simmons 2012-12-21 04:25:56 EST

We're trying to track down memory leaks introduced by switching our 3.x RCP app to eclipse 4.  I'm using MultiPageEditorPart as an example but I'm pretty sure its more general than this.  The WTP XML editor can be used as an example.

When a MultiPageEditorPart does a page change it does this:-

IPartService partService = (IPartService) getSite().getService(IPartService.class);

This ends up delegating to the PartSite's e4Context which is set to a ContextFunction that constructs a SlavePartService (see PartSite.initializeDefaultServices()).

The code it ends up executing in the function is

new SlavePartService(context.getParent().get(IPartService.class))

Now put a conditional break point in EclipseContext.addDependency(String, Computation) when

"org.eclipse.ui.IPartService".equals(name)

You'll see that the computation ends up being poked into the parent context - the perspective.  It is never cleaned up - it would be if it was in the context of the part.  You can verify this by putting a conditional breakpoint (same as above but now 'name' is a field) in

ValueComputation.handleInvalid(ContextChangeEvent, Set<Scheduled>)

The computation has a back-reference to the PartSite which typically leaks the editor model.

I think any of the other services would leak similarly if they delegate to the parent context like "context.getParent().get(IFooService.class)" and there are several in PartSite.


Its not surprising that its broken as all the EclipseContext instances can communicate via the static EclipseContext.currentComputation and end up adding dependencies on the current computation regardless of what EclipseContext said computation is actually associated with.

I can fix this leak locally as follows

1) Add method getOriginatingContext() to Computation (both subclasses have an EclipseContext in a field already).
2) Change EclipseContext.trackAccess(String) to add a dependency from the computation's originating context (obtained via getOriginatingContext()) rather than whatever contexts this is.

I have no idea whether this is the right approach though as I'm struggling to see what the code is trying to achieve.  I'd imagine that a better fix would be to get rid of the static EclipseContext.currentComputation all together.


NB I'll be on holiday over Christmas but I'm willing to do more to help here when I get back.

If you guys would be interesting in automated tests for memory leaks I have a suggestion.  We're using this:-

http://bits.netbeans.org/maven2/org/netbeans/modules/org-netbeans-insane/

Its part of the netbeans test framework.  Its cddl license but you only need use it during testing.  It lets you write unit tests that check that stuff becomes unreachable.