patch to 0.3.0 ExtensionFactoryMap for serious memory leak

[Chris Berry <ch...@gmail.com>]

Greetings,
Some time ago we reported a very serious memory leak in Abdera 0.3.0.
It was reported to the abdera users list on Nov 7, 2007 as "memory  
leak".
That message is replayed below...
At the time we could workaround the bug in our client, but over time  
this became impossible, and we had to hack ExtensionFactoryMap.

Attached is a patch to org.apache.abdera.factory.ExtensionFactoryMap
This patch simply removes the problem -- the WeakHashMap.
This change has a negligible effect on performance (as measured in  
load testing)
But without it, abdera 0.3.0 will get OutOfMemoryExceptions quite  
quickly under load, rendering it unusable.
BTW: this code has been in Production for many weeks...

We were hoping that you could apply this patch to 0.3.0??
Or better;  produce 0.3.1

We are not able to upgrade to 0.4.0 yet, and would rather not have a  
hacked copy of 0.3.0.
Also, others not able to upgrade yet (since the upgrade is not  
entirely transparent) may benefit from the patch

I will also open a JIRA on this.

Thanks,
-- Chris 


	From: 	bryon@jacob.net
	Subject: 	memory leak
	Date: 	November 5, 2007 3:49:24 PM CST
	To: 	abdera-user@incubator.apache.org
	Reply-To: 	abdera-user@incubator.apache.org


                     Hey All -

I'm working on a data service based on Abdera (working with Chris  
Berry, who's a regular on these lists...)  When we were running our  
first battery of serious load testing on our system, we encountered  
memory-leaky behavior, and a profiler showed us that we were indeed  
leaking hundreds of megabytes a minute, all traceable back to the  
wrappers field on org.apache.abdera.factory.ExtensionFactoryMap.  This  
field is a map from elements to their wrappers, if any.  At first, I  
was puzzled by the memory leak, as the field is initialized thusly:

	this.wrappers = Collections.synchronizedMap( new  
WeakHashMap<Element,Element>());

clearly, the implementor took care to make sure that this cache would  
not leak by making it a WeakHashMap, which generally guarantees that  
the map itself will not keep a key and its corresponding entry from  
being garbage collected.  I dug throughout our application code to  
find if we were actually holding other references to these objects,  
and I googled for anyone having problems with esoteric interactions  
between Collections.synchronizedMap and WeakHashMaps - found nothing  
there.  Then I went back to square one and re-read the WeakHashMap  
javadoc very carefully.  Here's the relevant section:

	Implementation note: The value objects in a WeakHashMap are held by  
ordinary strong references. Thus care should be taken to ensure that  
value objects do not strongly refer to their own keys, either directly  
or indirectly, since that will prevent the keys from being discarded.  
Note that a value object may refer indirectly to its key via the  
WeakHashMap itself; that is, a value object may strongly refer to some  
other key object whose associated value object, in turn, strongly  
refers to the key of the first value object. One way to deal with this  
is to wrap values themselves within WeakReferences before inserting,  
as in: m.put(key, new WeakReference(value)), and then unwrapping upon  
each get.

This is why there is a memory leak - the map is a mapping from  
elements to their wrappers - by the very nature of the object being a  
wrapper of the element, it will usually have a strong reference to the  
element itself, which is the key! You can verify that Abdera wrappers,  
in general, will do this by looking at  
org.apache.abdera.model.ElementWrapper, which takes the element being  
wrapped as its constructor argument, and holds a strong reference to  
it as an instance variable.

This map is an optimization to memoize the calls to  
getElementWrapper() and not reconstruct them more than is necessary -  
it is not needed for abdera to function properly, so we have  
temporarily worked around the problem in our own application like so -  
we used to acquire our FOMFactory by calling abdera.getFactory() on  
our org.apache.abdera.Abdera instance, and re-using that singleton  
throughout our application.  Now we construct a new FOMFactory with  
new FOMFactory(abdera) once per request to the server, and since the  
only appreciable state on the factory is this map itself, this is a  
valid work-around.

I'd initially planned to really fix this issue and submit a patch  
along with this message, but digging a little deeper, I'm not sure  
that the correct fix is crystal clear...  We could do as the javadoc  
above suggests, and wrap the values with WeakReferences to plug the  
leak, or we could use a LinkedHashMap configured as an LRU cache to  
just bound the cache, so it can't grow out of control - but right now,  
I don't think that either of those solutions would be correct, because  
it seems to me that none of the objects in the hierarchy rooted at  
FOMElement define equals() and/or hashCode() methods, so all of the  
objects are cached based on their actual object identity.  It seems  
that in the all likely use cases, instances of FOMElement and its  
descendants are re-parsed on every request to a server running abdera,  
and so what we will see is cache misses virtually 100% of the time, so  
even though we'd have plugged the leak, strictly speaking, we would  
have ignored the underlying issue that we're caching data on every  
request that will be fundamentally unable to be retrieved on  
subsequent requests.  This is based only on my reading over the code  
for a few hours, so I could be missing something, and I also might be  
forgetting about a use case that demands and makes proper use of this  
memoization, but as it stands right now, my recommended fix would  
probably be to just cut out the cache altogether, and allow for  
wrappers to get constructed fresh every time they are requested.  One  
more possibility is that the cache is actually a useful optimization,  
but only during the scope of one request - in which case the "work- 
around" we are using now is actually the best practice, and the fix  
would be to remove the factory instance on the Abdera class...

I'd like to hear from the Abdera developers what their thoughts are on  
this issue, and what the best resolution is likely to be.  This is no  
longer a pressing issue for our team, but it is potentially a time  
bomb waiting to blow up for any project dependent on Abdera.

thanks!  (and thanks for Abdera, generally - we're easily a year ahead  
of where we'd be on this project without it!)

-Bryon (long-time listener, first-time caller)

Re: patch to 0.3.0 ExtensionFactoryMap for serious memory leak

[Dan Diephouse <da...@mulesource.com>]

Yes, a 0.3.1 release would have to go through the whole incubator release
process. We shouldn't be updating the existing binaries. I know Apache
watches this, so even if we wanted to we'd get our hand seriously slapped.

On Tue, Apr 29, 2008 at 11:47 AM, Chris Berry <ch...@gmail.com> wrote:

> Thanks for responding Dan.
> I may be wrong, but (IIRC) I think 0.3.0 was patched a few times in place
> (not that its a best practice ;-)
> Would 0.3.1 have to go thru the same process as a minor patch release??
> If so,  it probably is too much work.
> Thanks,
> -- Chris
>
> On Apr 29, 2008, at 1:37 PM, Dan Diephouse wrote:
>
>  Hi Chris, due to the high amount of work it takes to get a release out of
> > the incubator, I really don't see this happening. I don't know what you mean
> > by applying this patch to 0.3.0, but we can't really go back and change the
> > release. It all has to go through the incubator process. And I'm going to
> > guess that if we're going through that process we're going to focus on
> > either 1.0-RC or 0.4.1.
> >
> > Dan
> >
> > Chris Berry wrote:
> >
> > > Greetings,
> > > Some time ago we reported a very serious memory leak in Abdera 0.3.0.
> > > It was reported to the abdera users list on Nov 7, 2007 as "memory
> > > leak".
> > > That message is replayed below...
> > > At the time we could workaround the bug in our client, but over time
> > > this became impossible, and we had to hack ExtensionFactoryMap.
> > >
> > > Attached is a patch to org.apache.abdera.factory.ExtensionFactoryMap
> > > This patch simply removes the problem -- the WeakHashMap.
> > > This change has a negligible effect on performance (as measured in
> > > load testing) But without it, abdera 0.3.0 will get OutOfMemoryExceptions
> > > quite quickly under load, rendering it unusable.
> > > BTW: this code has been in Production for many weeks...
> > >
> > > We were hoping that you could apply this patch to 0.3.0??
> > > Or better;  produce 0.3.1
> > >
> > > We are not able to upgrade to 0.4.0 yet, and would rather not have a
> > > hacked copy of 0.3.0.
> > > Also, others not able to upgrade yet (since the upgrade is not
> > > entirely transparent) may benefit from the patch
> > >
> > > I will also open a JIRA on this.
> > >
> > > Thanks,
> > > --
> > > Chris------------------------------------------------------------------------
> > >
> > >
> > > *From: * bryon@jacob.net <ma...@jacob.net>
> > >
> > > *Subject: * *memory leak*
> > >
> > > *Date: * November 5, 2007 3:49:24 PM CST
> > >
> > > *To: * abdera-user@incubator.apache.org <mailto:
> > > abdera-user@incubator.apache.org>
> > > *Reply-To: * abdera-user@incubator.apache.org <mailto:
> > > abdera-user@incubator.apache.org>
> > >
> > >
> > >                   Hey All -
> > >
> > > I'm working on a data service based on Abdera (working with Chris
> > > Berry, who's a regular on these lists...)  When we were running our first
> > > battery of serious load testing on our system, we encountered memory-leaky
> > > behavior, and a profiler showed us that we were indeed leaking hundreds of
> > > megabytes a minute, all traceable back to the wrappers field on
> > > org.apache.abdera.factory.ExtensionFactoryMap.  This field is a map from
> > > elements to their wrappers, if any.  At first, I was puzzled by the memory
> > > leak, as the field is initialized thusly:
> > >
> > > this.wrappers = Collections.synchronizedMap( new
> > > WeakHashMap<Element,Element>());
> > >
> > > clearly, the implementor took care to make sure that this cache would
> > > not leak by making it a WeakHashMap, which generally guarantees that the map
> > > itself will not keep a key and its corresponding entry from being garbage
> > > collected.  I dug throughout our application code to find if we were
> > > actually holding other references to these objects, and I googled for anyone
> > > having problems with esoteric interactions between
> > > Collections.synchronizedMap and WeakHashMaps - found nothing there.  Then I
> > > went back to square one and re-read the WeakHashMap javadoc very carefully.
> > >  Here's the relevant section:
> > >
> > > Implementation note: The value objects in a WeakHashMap are held by
> > > ordinary strong references. Thus care should be taken to ensure that value
> > > objects do not strongly refer to their own keys, either directly or
> > > indirectly, since that will prevent the keys from being discarded. Note that
> > > a value object may refer indirectly to its key via the WeakHashMap itself;
> > > that is, a value object may strongly refer to some other key object whose
> > > associated value object, in turn, strongly refers to the key of the first
> > > value object. One way to deal with this is to wrap values themselves within
> > > WeakReferences before inserting, as in: m.put(key, new
> > > WeakReference(value)), and then unwrapping upon each get.
> > >
> > > This is why there is a memory leak - the map is a mapping from
> > > elements to their wrappers - by the very nature of the object being a
> > > wrapper of the element, it will usually have a strong reference to the
> > > element itself, which is the key! You can verify that Abdera wrappers, in
> > > general, will do this by looking at org.apache.abdera.model.ElementWrapper,
> > > which takes the element being wrapped as its constructor argument, and holds
> > > a strong reference to it as an instance variable.
> > >
> > > This map is an optimization to memoize the calls to
> > > getElementWrapper() and not reconstruct them more than is necessary - it is
> > > not needed for abdera to function properly, so we have temporarily worked
> > > around the problem in our own application like so - we used to acquire our
> > > FOMFactory by calling abdera.getFactory() on our org.apache.abdera.Abdera
> > > instance, and re-using that singleton throughout our application.  Now we
> > > construct a new FOMFactory with new FOMFactory(abdera) once per request to
> > > the server, and since the only appreciable state on the factory is this map
> > > itself, this is a valid work-around.
> > >
> > > I'd initially planned to really fix this issue and submit a patch
> > > along with this message, but digging a little deeper, I'm not sure that the
> > > correct fix is crystal clear...  We could do as the javadoc above suggests,
> > > and wrap the values with WeakReferences to plug the leak, or we could use a
> > > LinkedHashMap configured as an LRU cache to just bound the cache, so it
> > > can't grow out of control - but right now, I don't think that either of
> > > those solutions would be correct, because it seems to me that none of the
> > > objects in the hierarchy rooted at FOMElement define equals() and/or
> > > hashCode() methods, so all of the objects are cached based on their actual
> > > object identity.  It seems that in the all likely use cases, instances of
> > > FOMElement and its descendants are re-parsed on every request to a server
> > > running abdera, and so what we will see is cache misses virtually 100% of
> > > the time, so even though we'd have plugged the leak, strictly speaking, we
> > > would have ignored the underlying issue that we're caching data on every
> > > request that will be fundamentally unable to be retrieved on subsequent
> > > requests.  This is based only on my reading over the code for a few hours,
> > > so I could be missing something, and I also might be forgetting about a use
> > > case that demands and makes proper use of this memoization, but as it stands
> > > right now, my recommended fix would probably be to just cut out the cache
> > > altogether, and allow for wrappers to get constructed fresh every time they
> > > are requested.  One more possibility is that the cache is actually a useful
> > > optimization, but only during the scope of one request - in which case the
> > > "work-around" we are using now is actually the best practice, and the fix
> > > would be to remove the factory instance on the Abdera class...
> > >
> > > I'd like to hear from the Abdera developers what their thoughts are on
> > > this issue, and what the best resolution is likely to be.  This is no longer
> > > a pressing issue for our team, but it is potentially a time bomb waiting to
> > > blow up for any project dependent on Abdera.
> > >
> > > thanks!  (and thanks for Abdera, generally - we're easily a year ahead
> > > of where we'd be on this project without it!)
> > >
> > > -Bryon (long-time listener, first-time caller)
> > >
> >
> >
> > --
> > Dan Diephouse
> > MuleSource
> > http://mulesource.com | http://netzooid.com
> >
>
>


-- 
Dan Diephouse
http://mulesource.com | http://netzooid.com/blog

Re: patch to 0.3.0 ExtensionFactoryMap for serious memory leak

[Chris Berry <ch...@gmail.com>]

Thanks for responding Dan.
I may be wrong, but (IIRC) I think 0.3.0 was patched a few times in  
place (not that its a best practice ;-)
Would 0.3.1 have to go thru the same process as a minor patch release??
If so,  it probably is too much work.
Thanks,
-- Chris 

On Apr 29, 2008, at 1:37 PM, Dan Diephouse wrote:

> Hi Chris, due to the high amount of work it takes to get a release  
> out of the incubator, I really don't see this happening. I don't  
> know what you mean by applying this patch to 0.3.0, but we can't  
> really go back and change the release. It all has to go through the  
> incubator process. And I'm going to guess that if we're going  
> through that process we're going to focus on either 1.0-RC or 0.4.1.
>
> Dan
>
> Chris Berry wrote:
>> Greetings,
>> Some time ago we reported a very serious memory leak in Abdera 0.3.0.
>> It was reported to the abdera users list on Nov 7, 2007 as "memory  
>> leak".
>> That message is replayed below...
>> At the time we could workaround the bug in our client, but over  
>> time this became impossible, and we had to hack ExtensionFactoryMap.
>>
>> Attached is a patch to org.apache.abdera.factory.ExtensionFactoryMap
>> This patch simply removes the problem -- the WeakHashMap.
>> This change has a negligible effect on performance (as measured in  
>> load testing) But without it, abdera 0.3.0 will get  
>> OutOfMemoryExceptions quite quickly under load, rendering it  
>> unusable.
>> BTW: this code has been in Production for many weeks...
>>
>> We were hoping that you could apply this patch to 0.3.0??
>> Or better;  produce 0.3.1
>>
>> We are not able to upgrade to 0.4.0 yet, and would rather not have  
>> a hacked copy of 0.3.0.
>> Also, others not able to upgrade yet (since the upgrade is not  
>> entirely transparent) may benefit from the patch
>>
>> I will also open a JIRA on this.
>>
>> Thanks,
>> -- Chris 
>> ------------------------------------------------------------------------
>>
>>
>> *From: * bryon@jacob.net <ma...@jacob.net>
>>
>> *Subject: * *memory leak*
>>
>> *Date: * November 5, 2007 3:49:24 PM CST
>>
>> *To: * abdera-user@incubator.apache.org <mailto:abdera-user@incubator.apache.org 
>> >
>> *Reply-To: * abdera-user@incubator.apache.org <mailto:abdera-user@incubator.apache.org 
>> >
>>
>>
>>                    Hey All -
>>
>> I'm working on a data service based on Abdera (working with Chris  
>> Berry, who's a regular on these lists...)  When we were running our  
>> first battery of serious load testing on our system, we encountered  
>> memory-leaky behavior, and a profiler showed us that we were indeed  
>> leaking hundreds of megabytes a minute, all traceable back to the  
>> wrappers field on org.apache.abdera.factory.ExtensionFactoryMap.   
>> This field is a map from elements to their wrappers, if any.  At  
>> first, I was puzzled by the memory leak, as the field is  
>> initialized thusly:
>>
>> this.wrappers = Collections.synchronizedMap( new  
>> WeakHashMap<Element,Element>());
>>
>> clearly, the implementor took care to make sure that this cache  
>> would not leak by making it a WeakHashMap, which generally  
>> guarantees that the map itself will not keep a key and its  
>> corresponding entry from being garbage collected.  I dug throughout  
>> our application code to find if we were actually holding other  
>> references to these objects, and I googled for anyone having  
>> problems with esoteric interactions between  
>> Collections.synchronizedMap and WeakHashMaps - found nothing  
>> there.  Then I went back to square one and re-read the WeakHashMap  
>> javadoc very carefully.  Here's the relevant section:
>>
>> Implementation note: The value objects in a WeakHashMap are held by  
>> ordinary strong references. Thus care should be taken to ensure  
>> that value objects do not strongly refer to their own keys, either  
>> directly or indirectly, since that will prevent the keys from being  
>> discarded. Note that a value object may refer indirectly to its key  
>> via the WeakHashMap itself; that is, a value object may strongly  
>> refer to some other key object whose associated value object, in  
>> turn, strongly refers to the key of the first value object. One way  
>> to deal with this is to wrap values themselves within  
>> WeakReferences before inserting, as in: m.put(key, new  
>> WeakReference(value)), and then unwrapping upon each get.
>>
>> This is why there is a memory leak - the map is a mapping from  
>> elements to their wrappers - by the very nature of the object being  
>> a wrapper of the element, it will usually have a strong reference  
>> to the element itself, which is the key! You can verify that Abdera  
>> wrappers, in general, will do this by looking at  
>> org.apache.abdera.model.ElementWrapper, which takes the element  
>> being wrapped as its constructor argument, and holds a strong  
>> reference to it as an instance variable.
>>
>> This map is an optimization to memoize the calls to  
>> getElementWrapper() and not reconstruct them more than is necessary  
>> - it is not needed for abdera to function properly, so we have  
>> temporarily worked around the problem in our own application like  
>> so - we used to acquire our FOMFactory by calling  
>> abdera.getFactory() on our org.apache.abdera.Abdera instance, and  
>> re-using that singleton throughout our application.  Now we  
>> construct a new FOMFactory with new FOMFactory(abdera) once per  
>> request to the server, and since the only appreciable state on the  
>> factory is this map itself, this is a valid work-around.
>>
>> I'd initially planned to really fix this issue and submit a patch  
>> along with this message, but digging a little deeper, I'm not sure  
>> that the correct fix is crystal clear...  We could do as the  
>> javadoc above suggests, and wrap the values with WeakReferences to  
>> plug the leak, or we could use a LinkedHashMap configured as an LRU  
>> cache to just bound the cache, so it can't grow out of control -  
>> but right now, I don't think that either of those solutions would  
>> be correct, because it seems to me that none of the objects in the  
>> hierarchy rooted at FOMElement define equals() and/or hashCode()  
>> methods, so all of the objects are cached based on their actual  
>> object identity.  It seems that in the all likely use cases,  
>> instances of FOMElement and its descendants are re-parsed on every  
>> request to a server running abdera, and so what we will see is  
>> cache misses virtually 100% of the time, so even though we'd have  
>> plugged the leak, strictly speaking, we would have ignored the  
>> underlying issue that we're caching data on every request that will  
>> be fundamentally unable to be retrieved on subsequent requests.   
>> This is based only on my reading over the code for a few hours, so  
>> I could be missing something, and I also might be forgetting about  
>> a use case that demands and makes proper use of this memoization,  
>> but as it stands right now, my recommended fix would probably be to  
>> just cut out the cache altogether, and allow for wrappers to get  
>> constructed fresh every time they are requested.  One more  
>> possibility is that the cache is actually a useful optimization,  
>> but only during the scope of one request - in which case the "work- 
>> around" we are using now is actually the best practice, and the fix  
>> would be to remove the factory instance on the Abdera class...
>>
>> I'd like to hear from the Abdera developers what their thoughts are  
>> on this issue, and what the best resolution is likely to be.  This  
>> is no longer a pressing issue for our team, but it is potentially a  
>> time bomb waiting to blow up for any project dependent on Abdera.
>>
>> thanks!  (and thanks for Abdera, generally - we're easily a year  
>> ahead of where we'd be on this project without it!)
>>
>> -Bryon (long-time listener, first-time caller)
>
>
> -- 
> Dan Diephouse
> MuleSource
> http://mulesource.com | http://netzooid.com

Re: patch to 0.3.0 ExtensionFactoryMap for serious memory leak

[Dan Diephouse <da...@mulesource.com>]

Hi Chris, due to the high amount of work it takes to get a release out 
of the incubator, I really don't see this happening. I don't know what 
you mean by applying this patch to 0.3.0, but we can't really go back 
and change the release. It all has to go through the incubator process. 
And I'm going to guess that if we're going through that process we're 
going to focus on either 1.0-RC or 0.4.1.

Dan

Chris Berry wrote:
> Greetings,
> Some time ago we reported a very serious memory leak in Abdera 0.3.0.
> It was reported to the abdera users list on Nov 7, 2007 as "memory leak".
> That message is replayed below...
> At the time we could workaround the bug in our client, but over time 
> this became impossible, and we had to hack ExtensionFactoryMap.
>
> Attached is a patch to org.apache.abdera.factory.ExtensionFactoryMap
> This patch simply removes the problem -- the WeakHashMap.
> This change has a negligible effect on performance (as measured in 
> load testing) 
> But without it, abdera 0.3.0 will get OutOfMemoryExceptions quite 
> quickly under load, rendering it unusable.
> BTW: this code has been in Production for many weeks...
>
> We were hoping that you could apply this patch to 0.3.0??
> Or better;  produce 0.3.1
>
> We are not able to upgrade to 0.4.0 yet, and would rather not have a 
> hacked copy of 0.3.0.
> Also, others not able to upgrade yet (since the upgrade is not 
> entirely transparent) may benefit from the patch
>
> I will also open a JIRA on this.
>
> Thanks,
> -- Chris 
>
> ------------------------------------------------------------------------
>
>
> *From: * bryon@jacob.net <ma...@jacob.net>
>
> *Subject: * *memory leak*
>
> *Date: * November 5, 2007 3:49:24 PM CST
>
> *To: * abdera-user@incubator.apache.org 
> <ma...@incubator.apache.org>
> *Reply-To: * abdera-user@incubator.apache.org 
> <ma...@incubator.apache.org>
>
>
>                     Hey All -
>
> I'm working on a data service based on Abdera (working with Chris 
> Berry, who's a regular on these lists...)  When we were running our 
> first battery of serious load testing on our system, we encountered 
> memory-leaky behavior, and a profiler showed us that we were indeed 
> leaking hundreds of megabytes a minute, all traceable back to the 
> wrappers field on org.apache.abdera.factory.ExtensionFactoryMap.  This 
> field is a map from elements to their wrappers, if any.  At first, I 
> was puzzled by the memory leak, as the field is initialized thusly:
>
> this.wrappers = Collections.synchronizedMap( new 
> WeakHashMap<Element,Element>());
>
> clearly, the implementor took care to make sure that this cache would 
> not leak by making it a WeakHashMap, which generally guarantees that 
> the map itself will not keep a key and its corresponding entry from 
> being garbage collected.  I dug throughout our application code to 
> find if we were actually holding other references to these objects, 
> and I googled for anyone having problems with esoteric interactions 
> between Collections.synchronizedMap and WeakHashMaps - found nothing 
> there.  Then I went back to square one and re-read the WeakHashMap 
> javadoc very carefully.  Here's the relevant section:
>
> Implementation note: The value objects in a WeakHashMap are held by 
> ordinary strong references. Thus care should be taken to ensure that 
> value objects do not strongly refer to their own keys, either directly 
> or indirectly, since that will prevent the keys from being discarded. 
> Note that a value object may refer indirectly to its key via the 
> WeakHashMap itself; that is, a value object may strongly refer to some 
> other key object whose associated value object, in turn, strongly 
> refers to the key of the first value object. One way to deal with this 
> is to wrap values themselves within WeakReferences before inserting, 
> as in: m.put(key, new WeakReference(value)), and then unwrapping upon 
> each get.
>
> This is why there is a memory leak - the map is a mapping from 
> elements to their wrappers - by the very nature of the object being a 
> wrapper of the element, it will usually have a strong reference to the 
> element itself, which is the key! You can verify that Abdera wrappers, 
> in general, will do this by looking at 
> org.apache.abdera.model.ElementWrapper, which takes the element being 
> wrapped as its constructor argument, and holds a strong reference to 
> it as an instance variable.
>
> This map is an optimization to memoize the calls to 
> getElementWrapper() and not reconstruct them more than is necessary - 
> it is not needed for abdera to function properly, so we have 
> temporarily worked around the problem in our own application like so - 
> we used to acquire our FOMFactory by calling abdera.getFactory() on 
> our org.apache.abdera.Abdera instance, and re-using that singleton 
> throughout our application.  Now we construct a new FOMFactory with 
> new FOMFactory(abdera) once per request to the server, and since the 
> only appreciable state on the factory is this map itself, this is a 
> valid work-around.
>
> I'd initially planned to really fix this issue and submit a patch 
> along with this message, but digging a little deeper, I'm not sure 
> that the correct fix is crystal clear...  We could do as the javadoc 
> above suggests, and wrap the values with WeakReferences to plug the 
> leak, or we could use a LinkedHashMap configured as an LRU cache to 
> just bound the cache, so it can't grow out of control - but right now, 
> I don't think that either of those solutions would be correct, because 
> it seems to me that none of the objects in the hierarchy rooted at 
> FOMElement define equals() and/or hashCode() methods, so all of the 
> objects are cached based on their actual object identity.  It seems 
> that in the all likely use cases, instances of FOMElement and its 
> descendants are re-parsed on every request to a server running abdera, 
> and so what we will see is cache misses virtually 100% of the time, so 
> even though we'd have plugged the leak, strictly speaking, we would 
> have ignored the underlying issue that we're caching data on every 
> request that will be fundamentally unable to be retrieved on 
> subsequent requests.  This is based only on my reading over the code 
> for a few hours, so I could be missing something, and I also might be 
> forgetting about a use case that demands and makes proper use of this 
> memoization, but as it stands right now, my recommended fix would 
> probably be to just cut out the cache altogether, and allow for 
> wrappers to get constructed fresh every time they are requested.  One 
> more possibility is that the cache is actually a useful optimization, 
> but only during the scope of one request - in which case the 
> "work-around" we are using now is actually the best practice, and the 
> fix would be to remove the factory instance on the Abdera class...
>
> I'd like to hear from the Abdera developers what their thoughts are on 
> this issue, and what the best resolution is likely to be.  This is no 
> longer a pressing issue for our team, but it is potentially a time 
> bomb waiting to blow up for any project dependent on Abdera.
>
> thanks!  (and thanks for Abdera, generally - we're easily a year ahead 
> of where we'd be on this project without it!)
>
> -Bryon (long-time listener, first-time caller)


-- 
Dan Diephouse
MuleSource
http://mulesource.com | http://netzooid.com