Bug 455572 - memory leak due to incorrect handling of XML load options
Summary: memory leak due to incorrect handling of XML load options
Status: NEW
Alias: None
Product: MDT.UML2
Classification: Modeling
Component: Core (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows NT
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: UML2 Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-18 04:00 EST by Laurent Goubet CLA
Modified: 2015-02-28 21:16 EST (History)
3 users (show)

See Also:


Attachments
unit test (11.24 KB, application/zip)
2014-12-18 04:00 EST, Laurent Goubet CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Laurent Goubet CLA 2014-12-18 04:00:58 EST
Created attachment 249515 [details]
unit test

I could find no mention of this anywhere, but UML2 causes issues when using some of the load options available for the EMF resource. Namely, XMLResource.OPTION_DISABLE_NOTIFY breaks quite a few behaviors.

One of the things it does is that the CacheAdapter will create unreachable and uncleanable proxies when using this option to load the models. On a very little model such as what I have in the attached unit test, this is only a single proxy. Though since we're talking about the CacheAdapter, this amounts to multiple Strings and collections in memory. On one of the large, well fragmented models we use here, this amounts to 300M of heap memory that _cannot_ be recovered (see @Before of the attached unit test to see how this needs to be cleaned up).

This unit test also highlights how CacheAdapter#getProxyMap() simply fails in NPE in most cases... that should probably be fixed too.

The problematic behavior comes from the interaction of two behaviors :
1. the implementation of ElementImpl#eSetDeliver(boolean) that will eagerly handle all cross-references and create proxies for them, even if it points to another resource, and
2. the fact that the cache adapter will never clear its proxies if the resource that contain them is not loaded (which, btw, is the point of a proxy in the first place).

As you will see in debug, the proxy is created through ElementImpl#eSetDeliver() that is called from org.eclipse.emf.ecore.xmi.impl.XMLHandler.endDocument() since "disableNotify" is true. And that proxy will never be cleared up since I've never loaded the resource that contains it in memory; I've just loaded the first, "root" model, model.uml and then unloaded it once I was done with it, never triggering the loading of statemachine.uml (that contains the object pointed to by the proxy).

note : I've always run the attached test as a plugin test, no idea if this works as a simple Junit test.
Comment 1 Ed Willink CLA 2014-12-18 04:27:37 EST
I suspect this falls in the same category as the GenModel option to suppress generation of the resolveProxies bloat for simple models. It just isn't supported for non-trivial models. Perhaps there could be a warning message during load.
Comment 2 Kenn Hussey CLA 2014-12-22 09:43:13 EST
(In reply to Laurent Goubet from comment #0)
> This unit test also highlights how CacheAdapter#getProxyMap() simply fails
> in NPE in most cases... that should probably be fixed too.

I'll fix this (to return null instead of throwing an exception).

> The problematic behavior comes from the interaction of two behaviors :
> 1. the implementation of ElementImpl#eSetDeliver(boolean) that will eagerly
> handle all cross-references and create proxies for them, even if it points
> to another resource, and
> 2. the fact that the cache adapter will never clear its proxies if the
> resource that contain them is not loaded (which, btw, is the point of a
> proxy in the first place).

This behaviour comes from EMF and is not unique to the cache adapter (or to UML2).

> As you will see in debug, the proxy is created through
> ElementImpl#eSetDeliver() that is called from
> org.eclipse.emf.ecore.xmi.impl.XMLHandler.endDocument() since
> "disableNotify" is true. And that proxy will never be cleared up since I've
> never loaded the resource that contains it in memory; I've just loaded the
> first, "root" model, model.uml and then unloaded it once I was done with it,
> never triggering the loading of statemachine.uml (that contains the object
> pointed to by the proxy).

If the cached proxies aren’t being cleared when the referring resource is unloaded, that is a bug and I suspect it applies to generic cross reference adapters (i.e., ECrossReferenceAdapter) as well. Are you able to verify whether that is the case?
Comment 3 Laurent Goubet CLA 2015-01-05 03:19:18 EST
(In reply to Kenn Hussey from comment #2)
> > As you will see in debug, the proxy is created through
> > ElementImpl#eSetDeliver() that is called from
> > org.eclipse.emf.ecore.xmi.impl.XMLHandler.endDocument() since
> > "disableNotify" is true. And that proxy will never be cleared up since I've
> > never loaded the resource that contains it in memory; I've just loaded the
> > first, "root" model, model.uml and then unloaded it once I was done with it,
> > never triggering the loading of statemachine.uml (that contains the object
> > pointed to by the proxy).
> 
> If the cached proxies aren’t being cleared when the referring resource is
> unloaded, that is a bug and I suspect it applies to generic cross reference
> adapters (i.e., ECrossReferenceAdapter) as well. Are you able to verify
> whether that is the case?

We use a number of ECrossReferenceAdapters and I've never observed this memory leak, I believe I'd remember given how hard the source cause was to pinpoint.

That said, it can pretty easily be tested by deleting the fragment model (statemachine.uml) and installing a standard cross referencer adapter on the root. This should be enough for the proxy to be seen (since EMF won't be able to load the fragment) and referenced by the adapter.
Comment 4 Kenn Hussey CLA 2015-02-28 21:16:56 EST
(In reply to Kenn Hussey from comment #2)
> (In reply to Laurent Goubet from comment #0)
> > This unit test also highlights how CacheAdapter#getProxyMap() simply fails
> > in NPE in most cases... that should probably be fixed too.
> 
> I'll fix this (to return null instead of throwing an exception).
> 

A fix for this has been committed/pushed to the ‘master’ branch in git and will be available in the next build of UML2 5.1.0.

(In reply to Laurent Goubet from comment #3)
> (In reply to Kenn Hussey from comment #2)
> > If the cached proxies aren’t being cleared when the referring resource is
> > unloaded, that is a bug and I suspect it applies to generic cross reference
> > adapters (i.e., ECrossReferenceAdapter) as well. Are you able to verify
> > whether that is the case?
> 
> We use a number of ECrossReferenceAdapters and I've never observed this
> memory leak, I believe I'd remember given how hard the source cause was to
> pinpoint.
> 
> That said, it can pretty easily be tested by deleting the fragment model
> (statemachine.uml) and installing a standard cross referencer adapter on the
> root. This should be enough for the proxy to be seen (since EMF won't be
> able to load the fragment) and referenced by the adapter.

I’ve finally had a chance to test this myself and have confirmed that the bug does indeed exist in EMF as well. If you add a specialization of ECrossReferenceAdapter which returns ‘false’ for resolve() to a resource set, it will leak objects in the proxy map. If the XMLResource.OPTION_DISABLE_NOTIFY load option is used, you simply need to add the adapter to the resource set after the resource has been loaded to see the problem, but without that option it doesn’t matter when the adapter is added.

I suspect this problem may be unique to containment proxies - the (containment) proxy is added to the cross referencer (and proxy map) when the adapter is attached (as it traverses the containment hierarchy) but it isn’t removed from the proxy map when the adapter is unattached, presumably because containment references are ignored when handling cross references (and so aren’t processed when removing the adapter).