You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by Daniel Kulp <dk...@apache.org> on 2008/06/01 03:49:57 UTC
Re: Memory Leak at WSDLManagerImpl
Hmmm.... good sleuthing.
You definitely don't want to unregister it on endpoint stop or when
done with the client. The whole point of the cache is to hold onto
it so it's reusable. On popular thing to do is create a client, use
it once, discard. Create another client, use it once, discard.
Etc... The cache is to help that.
I think what might make some sense is to get rid of the schemaCacheMap
entirely. Change the definitionsMap to be a Map<Object, DefPair> map
where DepPair holds a wsdldef and the schema. Thus, the keys would
be the same keys for the wsdl.
The put/set's would be more expensive as you would need to iterate
through the cache to find the pair. I doubt the map would be very big
though so that may be acceptable.
Dan
On May 31, 2008, at 4:55 PM, Bharath Ganesh wrote:
> I figured out a memory leak at WSDLManagerImpl schemaCacheMap.
> The schemaCacheMap here has a weak key - WSDLDefinition and value
> ServiceSchemaInfo. A key,value pair is inserted into this map while
> building
> a service. The entry is never explicitly removed from this map.
> Since the
> map is a WeakHashMap, it is assumed that when the key WSDLDefinition
> is
> weakly referenced, the entry would be removed from the map. But it
> is seen
> that the value ServiceSchemaInfo(the value) holds on to a SchemaInfo
> which
> in turn holds on to the ServiceInfo, which holds the WSDLDefinition
> through
> the AbstractPropertiesHolder.
> This would mean that the value of the CacheMap always strongly
> refers to the
> key, which would mean the entry would never be removed from the map.
> "*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"
>
> *This would mean ServiceInfo, OperationInfo, BindingInfo,
> WSDLDefinition
> would all stay in the VM even after the endpoint is stopped.
>
> One solution I could think of was to explicitly remove the entry on
> endpoint
> stop, instead of relying on the WeakHashMap semantics. This would
> work fine
> for server endpoints. But Is there any way to do so for client
> endpoints?
---
Daniel Kulp
dkulp@apache.org
http://www.dankulp.com/blog
Re: Memory Leak at WSDLManagerImpl
Posted by Daniel Kulp <dk...@apache.org>.
On Jun 2, 2008, at 9:17 AM, Bharath Ganesh wrote:
> I have verified the GC with the below fix(schemaCacheMap has a weak
> value)
> and the literal String fix(now always use the url at
> WSDLServiceFactory as
> key).
>
> After an application undeployment, things look fine on the Java EE
> Server.
> Except that the entry in JAXBCONTEXT_CACHE in JAXBDataBinding would
> be still
> retained till the next endpoint deployment/undeployment - WeakHashMap
> expunges stale entries only on map access. Thus the classloader and
> JAXB
> classes of the undeployed application would still be on the heap
> till the
> next app deployment. This is not a memory leak. Later we can think
> of a
> daemon thread which will access this map periodically to force stale
> entry
> expunging.
>
> Will commit these changes on 2.1 branch. DanK hope you will apply on
> 2.0.7
> also.
Definitely. Once they are there on 2.1, I'll port them over to
2.0.7. Great work.
Dan
>
>
>
> On Mon, Jun 2, 2008 at 11:55 AM, Bharath Ganesh <bharathganesh@gmail.com
> >
> wrote:
>
>
>> Yes we would need to iterate through the map while putting/getting
>> the
>> ServiceSchemaInfo.
>> Do you think we should instead choose to retain the schemaCacheMap,
>> but
>> make the value always a weak object. (Putting a WeakReference of the
>> ServiceSchemaInfo in the map). This way also we can solve the current
>> problem.
>> What do you say?
>>
>>
>>
>> On Sun, Jun 1, 2008 at 7:19 AM, Daniel Kulp <dk...@apache.org> wrote:
>>
>>>
>>> Hmmm.... good sleuthing.
>>>
>>> You definitely don't want to unregister it on endpoint stop or
>>> when done
>>> with the client. The whole point of the cache is to hold onto it
>>> so it's
>>> reusable. On popular thing to do is create a client, use it once,
>>> discard.
>>> Create another client, use it once, discard. Etc... The cache is
>>> to help
>>> that.
>>>
>>> I think what might make some sense is to get rid of the
>>> schemaCacheMap
>>> entirely. Change the definitionsMap to be a Map<Object, DefPair>
>>> map where
>>> DepPair holds a wsdldef and the schema. Thus, the keys would be
>>> the same
>>> keys for the wsdl.
>>>
>>> The put/set's would be more expensive as you would need to iterate
>>> through
>>> the cache to find the pair. I doubt the map would be very big
>>> though so
>>> that may be acceptable.
>>>
>>> Dan
>>>
>>>
>>>
>>>
>>> On May 31, 2008, at 4:55 PM, Bharath Ganesh wrote:
>>>
>>> I figured out a memory leak at WSDLManagerImpl schemaCacheMap.
>>>> The schemaCacheMap here has a weak key - WSDLDefinition and value
>>>> ServiceSchemaInfo. A key,value pair is inserted into this map while
>>>> building
>>>> a service. The entry is never explicitly removed from this map.
>>>> Since the
>>>> map is a WeakHashMap, it is assumed that when the key
>>>> WSDLDefinition is
>>>> weakly referenced, the entry would be removed from the map. But
>>>> it is
>>>> seen
>>>> that the value ServiceSchemaInfo(the value) holds on to a
>>>> SchemaInfo
>>>> which
>>>> in turn holds on to the ServiceInfo, which holds the WSDLDefinition
>>>> through
>>>> the AbstractPropertiesHolder.
>>>> This would mean that the value of the CacheMap always strongly
>>>> refers to
>>>> the
>>>> key, which would mean the entry would never be removed from the
>>>> map.
>>>> "*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"
>>>>
>>>> *This would mean ServiceInfo, OperationInfo, BindingInfo,
>>>> WSDLDefinition
>>>> would all stay in the VM even after the endpoint is stopped.
>>>>
>>>> One solution I could think of was to explicitly remove the entry on
>>>> endpoint
>>>> stop, instead of relying on the WeakHashMap semantics. This
>>>> would work
>>>> fine
>>>> for server endpoints. But Is there any way to do so for client
>>>> endpoints?
>>>>
>>>
>>> ---
>>> Daniel Kulp
>>> dkulp@apache.org
>>> http://www.dankulp.com/blog
>>>
>>>
>>>
>>>
>>>
>>
---
Daniel Kulp
dkulp@apache.org
http://www.dankulp.com/blog
Re: Memory Leak at WSDLManagerImpl
Posted by Bharath Ganesh <bh...@gmail.com>.
I have verified the GC with the below fix(schemaCacheMap has a weak value)
and the literal String fix(now always use the url at WSDLServiceFactory as
key).
After an application undeployment, things look fine on the Java EE Server.
Except that the entry in JAXBCONTEXT_CACHE in JAXBDataBinding would be still
retained till the next endpoint deployment/undeployment - WeakHashMap
expunges stale entries only on map access. Thus the classloader and JAXB
classes of the undeployed application would still be on the heap till the
next app deployment. This is not a memory leak. Later we can think of a
daemon thread which will access this map periodically to force stale entry
expunging.
Will commit these changes on 2.1 branch. DanK hope you will apply on 2.0.7
also.
On Mon, Jun 2, 2008 at 11:55 AM, Bharath Ganesh <bh...@gmail.com>
wrote:
> Yes we would need to iterate through the map while putting/getting the
> ServiceSchemaInfo.
> Do you think we should instead choose to retain the schemaCacheMap, but
> make the value always a weak object. (Putting a WeakReference of the
> ServiceSchemaInfo in the map). This way also we can solve the current
> problem.
> What do you say?
>
>
>
> On Sun, Jun 1, 2008 at 7:19 AM, Daniel Kulp <dk...@apache.org> wrote:
>
>>
>> Hmmm.... good sleuthing.
>>
>> You definitely don't want to unregister it on endpoint stop or when done
>> with the client. The whole point of the cache is to hold onto it so it's
>> reusable. On popular thing to do is create a client, use it once, discard.
>> Create another client, use it once, discard. Etc... The cache is to help
>> that.
>>
>> I think what might make some sense is to get rid of the schemaCacheMap
>> entirely. Change the definitionsMap to be a Map<Object, DefPair> map where
>> DepPair holds a wsdldef and the schema. Thus, the keys would be the same
>> keys for the wsdl.
>>
>> The put/set's would be more expensive as you would need to iterate through
>> the cache to find the pair. I doubt the map would be very big though so
>> that may be acceptable.
>>
>> Dan
>>
>>
>>
>>
>> On May 31, 2008, at 4:55 PM, Bharath Ganesh wrote:
>>
>> I figured out a memory leak at WSDLManagerImpl schemaCacheMap.
>>> The schemaCacheMap here has a weak key - WSDLDefinition and value
>>> ServiceSchemaInfo. A key,value pair is inserted into this map while
>>> building
>>> a service. The entry is never explicitly removed from this map. Since the
>>> map is a WeakHashMap, it is assumed that when the key WSDLDefinition is
>>> weakly referenced, the entry would be removed from the map. But it is
>>> seen
>>> that the value ServiceSchemaInfo(the value) holds on to a SchemaInfo
>>> which
>>> in turn holds on to the ServiceInfo, which holds the WSDLDefinition
>>> through
>>> the AbstractPropertiesHolder.
>>> This would mean that the value of the CacheMap always strongly refers to
>>> the
>>> key, which would mean the entry would never be removed from the map.
>>> "*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"
>>>
>>> *This would mean ServiceInfo, OperationInfo, BindingInfo, WSDLDefinition
>>> would all stay in the VM even after the endpoint is stopped.
>>>
>>> One solution I could think of was to explicitly remove the entry on
>>> endpoint
>>> stop, instead of relying on the WeakHashMap semantics. This would work
>>> fine
>>> for server endpoints. But Is there any way to do so for client endpoints?
>>>
>>
>> ---
>> Daniel Kulp
>> dkulp@apache.org
>> http://www.dankulp.com/blog
>>
>>
>>
>>
>>
>
Re: Memory Leak at WSDLManagerImpl
Posted by Bharath Ganesh <bh...@gmail.com>.
Yes we would need to iterate through the map while putting/getting the
ServiceSchemaInfo.
Do you think we should instead choose to retain the schemaCacheMap, but make
the value always a weak object. (Putting a WeakReference of the
ServiceSchemaInfo in the map). This way also we can solve the current
problem.
What do you say?
On Sun, Jun 1, 2008 at 7:19 AM, Daniel Kulp <dk...@apache.org> wrote:
>
> Hmmm.... good sleuthing.
>
> You definitely don't want to unregister it on endpoint stop or when done
> with the client. The whole point of the cache is to hold onto it so it's
> reusable. On popular thing to do is create a client, use it once, discard.
> Create another client, use it once, discard. Etc... The cache is to help
> that.
>
> I think what might make some sense is to get rid of the schemaCacheMap
> entirely. Change the definitionsMap to be a Map<Object, DefPair> map where
> DepPair holds a wsdldef and the schema. Thus, the keys would be the same
> keys for the wsdl.
>
> The put/set's would be more expensive as you would need to iterate through
> the cache to find the pair. I doubt the map would be very big though so
> that may be acceptable.
>
> Dan
>
>
>
>
> On May 31, 2008, at 4:55 PM, Bharath Ganesh wrote:
>
> I figured out a memory leak at WSDLManagerImpl schemaCacheMap.
>> The schemaCacheMap here has a weak key - WSDLDefinition and value
>> ServiceSchemaInfo. A key,value pair is inserted into this map while
>> building
>> a service. The entry is never explicitly removed from this map. Since the
>> map is a WeakHashMap, it is assumed that when the key WSDLDefinition is
>> weakly referenced, the entry would be removed from the map. But it is seen
>> that the value ServiceSchemaInfo(the value) holds on to a SchemaInfo which
>> in turn holds on to the ServiceInfo, which holds the WSDLDefinition
>> through
>> the AbstractPropertiesHolder.
>> This would mean that the value of the CacheMap always strongly refers to
>> the
>> key, which would mean the entry would never be removed from the map.
>> "*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"
>>
>> *This would mean ServiceInfo, OperationInfo, BindingInfo, WSDLDefinition
>> would all stay in the VM even after the endpoint is stopped.
>>
>> One solution I could think of was to explicitly remove the entry on
>> endpoint
>> stop, instead of relying on the WeakHashMap semantics. This would work
>> fine
>> for server endpoints. But Is there any way to do so for client endpoints?
>>
>
> ---
> Daniel Kulp
> dkulp@apache.org
> http://www.dankulp.com/blog
>
>
>
>
>