You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openjpa.apache.org by Kevan Miller <ke...@gmail.com> on 2007/07/16 23:31:45 UTC
PCRegistry ClassLoader memory leak
Geronimo is running out of PermGen space in some simple deploy/
undeploy scenarios involving OpenJPA. The cause of the problem seems
to be the _metas table in PCRegistry. _metas is a
ConcurrentReferenceHashMap with WEAK reference keys and HARD
reference values. The keys are the PersistenceCapable classes. While
the values are the metadata for these classes which are maintained by
the internal Meta class.
The cause of the ClassLoader memory leak is simple -- if any of the
objects/classes held by the Meta class (e.g. fieldTypes) have also
been loaded by the same ClassLoader used to load the
PersistenceCapable class, the PersistenceCapable class (the weak key)
will never be GCed. The value of the HashMap entry will always
maintain a hard reference to the ClassLoader. Since the ClassLoader
will never be GC'ed, the the the pcClass Class object will never be
GC'able...
The problem can be easily recreated using current Geronimo trunk and
the Geronimo Daytrader application.
--kevan
Re: PCRegistry ClassLoader memory leak
Posted by Kevan Miller <ke...@gmail.com>.
I've attached two patches to https://issues.apache.org/jira/browse/
OPENJPA-285. I've tested deploy/undeploy scenarios with the two
patches. They appear to clear up ClassLoader memory leaks associated
with deployment/undeployment of applications.
The patch to ImplHelper is straight-forward, IMO. _assignableTypes
maintains a Map of Class, Map pairs. The Class key is WEAK. The
assignableTo Maps need to use WEAK keys, also. Otherwise, there is a
chain of HARD references which will prevent GC of the 'to' Class'
ClassLoader.
The PCRegistry change was the subject of earlier discussion. I see no
mechanism of WeakReferences/Stringified object names that would
permit the appropriate Garbage Collection of the Meta.pc object (and
it's associated ClassLoader). Instead, this patch adds a
PCRegistry.deregister(ClassLoader) method. When a ClassLoader is no
longer viable, Embedders may use this method to cause PCRegistry to
remove Meta objects from the _metas Map.
My testing shows that these two patches clear up the ClassLoader
memory leaks (at least in the scenarios I've tested). Hoping that we
can get these patches applied...
Thanks!
--kevan
Re: PCRegistry ClassLoader memory leak
Posted by Kevan Miller <ke...@gmail.com>.
On Jul 16, 2007, at 5:40 PM, Marc Prud'hommeaux wrote:
> Kevan-
>
> How many deploy/undeploys does it take before it runs out of memory?
Hi Marc,
It's application dependent. I think it took about 20 deploy/undeploy
cycles. We're losing a ClassLoader per deploy/undeploy. Looks like
for this app each ClassLoader is ~ 1 meg. I was running with an 80meg
max permgen:
export JAVA_OPTS="-XX:MaxPermSize=80m -verbose:gc -XX:+PrintGCDetails
-XX:+HeapDumpOnOutOfMemoryError"
FYI, here are the GC Roots for one of our ClassLoaders being leaked
-- http://people.apache.org/~kevan/PCRegistryLeak.html
>
> I recall a long time back we had a similar problem (although the
> error was entrenched in the same functionality contained in the
> javax.jdo.spi.JDOImplHelper. I don't know if a solution was ever
> found. Short of figuring out some way to listen for the death of a
> ClassLoader and manually unregistering metadata when that happens,
> I can't think of any way to deal with this automatically.
>
> Anyone have any ideas?
Well, with some increased complexity, you could have WeakReference
values (have ConcurrentReferenceHashMap permit WeakReference keys and
WeakReference values (or wrap Meta with a WeakReference before
inserting into the _metas table).
Define an OpenJPA specific interface for triggering ClassLoader
events. Either a callback interface or straight method call. The JSF
specification defined a javax.faces.FactoryFinder.releaseFactories()
method which can be called during an undeploy. That's spec-defined
however. I'd prefer not to go that way, but it's not impossible...
--kevan
Re: PCRegistry ClassLoader memory leak
Posted by Sahoo <Sa...@Sun.COM>.
robert burrell donkin wrote:
> On 7/17/07, Marina Vatkina <Ma...@sun.com> wrote:
>> I'm wondering if the EMF.close() either called by the user or by the
>> container,
>> when the application is unloaded/undeployed can trigger the cleanup?
>
> JEE unfortunately lacks lifecycle methods
>
Lifecycle of the container managed EMF is defined in the JPA spec in
section #7.1.1. Those rules don't apply for Java SE style of JPA in a
Java EE container. If that's a web app, then it should be able to use
something like *ServletContextListener.contextDestroyted*. I don't know
any such facilities for EJB container.
Thanks,
Sahoo
Re: PCRegistry ClassLoader memory leak
Posted by robert burrell donkin <ro...@gmail.com>.
On 7/17/07, Marina Vatkina <Ma...@sun.com> wrote:
> I'm wondering if the EMF.close() either called by the user or by the container,
> when the application is unloaded/undeployed can trigger the cleanup?
JEE unfortunately lacks lifecycle methods
- robert
Re: PCRegistry ClassLoader memory leak
Posted by Marc Prud'hommeaux <mp...@apache.org>.
> If the only time the WeakReference will be nullified is when the
> class is no longer referenced, isn't it ok to use WeakReference?
> What are the circumstances under which the class is not referenced?
> If its class loader is garbage collected, or no class references
> the pc class?
>
> The Registry need only keep track of currently used classes. If the
> class loader decides to unload the class, it's ok to remove it from
> the registry, as long as when the class loader needs it again it
> initializes it.
The Meta.pc problem is with the *instance* of the class that is being
kept around for pcNewInstance() purposes. If you have a WeakMap of
SomeClass : Foo->Bar->InstanceOfSomeClass, then the SomeClass key can
never be cleaned up. And if InstanceOfSomeClass is made weak, then it
might be garbage collected.
> It's been a long time since this architecture was put into place
> but I don't recall any real issues like these...
I submitted a bug report to Sun ("Incident Review ID: 261116") about
this back in 2004. I believe there was also some discussion of it
back in March of 2006 on the EG list.
On Jul 18, 2007, at 4:11 PM, Craig L Russell wrote:
>
> On Jul 18, 2007, at 3:51 PM, Marc Prud'hommeaux wrote:
>
>>
>> On Jul 18, 2007, at 2:52 PM, Kevan Miller wrote:
>>
>>>
>>> On Jul 17, 2007, at 5:32 PM, Marc Prud'hommeaux wrote:
>>>
>>>>
>>>> Correct me if I am wrong, but I think the problem is essentially
>>>> that we have a Map of SomeClass:Object->Object->...->SomeClass,
>>>> and therefore SomeClass can never be garbage collected,
>>>> regardless of whether the keys in the Map are soft or weak, and
>>>> regardless of whether the class' ClassLoader is referenced
>>>> elsewhere or not.
>>>>
>>>> Another possible solution might be to find every place where we
>>>> have a Class field that could possibly be set to a
>>>> PersistenceCapable, and turn them all into a
>>>> WeakReference<Class>. I haven't checked to see how big a change
>>>> that would be, but it might be one of the simpler solutions (and
>>>> it might be faster than storing the class name as a String and
>>>> doing a Class.forName() every time we need access to the Class
>>>> object).
>>>
>>> There are two categories of references that are keeping
>>> ClassLoaders alive.
>>>
>>> (1) References to classes (i.e. Meta fields pcClass, fieldTypes
>>> [], and pcSuper). These can all be wrapped fairly easily using
>>> WeakReferences. I've fixed these, locally.
>>
>> Sounds good.
>>
>>> (2) References to objects (i.e Meta.pc). This is a problem.
>>> Meta.pc is the only reference to the PersistenceCapable object.
>>> Since Meta.pc is the only reference to the PersistenceCapable
>>> object, it can't be a WeakReference.
>
> If the only time the WeakReference will be nullified is when the
> class is no longer referenced, isn't it ok to use WeakReference?
> What are the circumstances under which the class is not referenced?
> If its class loader is garbage collected, or no class references
> the pc class?
>
> The Registry need only keep track of currently used classes. If the
> class loader decides to unload the class, it's ok to remove it from
> the registry, as long as when the class loader needs it again it
> initializes it.
>
> It's been a long time since this architecture was put into place
> but I don't recall any real issues like these...
>
> Craig
>>>
>>> Any thoughts on fixing the problem with Meta.pc? Does it need to
>>> be an object? Can we generate an instance of PersistenceCapable
>>> as needed?
>>
>> That is unfortunate. To my knowledge, the only reason we need to
>> keep around an instance of the object is that it is easy to
>> construct new instances using PersistenceCapable.pcNewInstance()
>> without having to use reflection each time or worry about
>> security. I'm not sure of any way around this.
>>
>>> Or, on a different tack, can PCRegistry be made non-static?
>>> Associated with the PU, for instance?
>>
>> I personally think it would be nice if we could get rid of
>> PCRegistry being static. However, this part of the problem with
>> the Meta.pc issue in the previous paragraph: the static
>> initializer of any enhanced persistent class will do:
>>
>> PCRegistry.register(MyClass.class, pcFieldNames, pcFieldTypes,
>> pcFieldFlags, pcPCSuperclass, "MyClass", new MyClass());
>>
>> If PCRegistry weren't static, then we wouldn't be able to do this.
>>
>> Also, I think there are a number of places where we rely on
>> PCRegistry being static for other reasons. I think it would be
>> nice to get rid of these, but that project might be too large a
>> scope just to fix this bug.
>>
>>
>>> As Patrick mentions, another approach is to load OpenJPA in every
>>> application classloader (or a unique OpenJPA ClassLoader per
>>> application ClassLoader). I really don't want to do this... I'd
>>> prefer a private interface to notify you of when ClassLoaders are
>>> no longer valid...
>>>
>>> --kevan
>>>
>>>
>>>>
>>>>
>>>> On Jul 17, 2007, at 10:14 AM, Patrick Linskey wrote:
>>>>
>>>>> Of course, another approach is for Geronimo to trash the
>>>>> classloader
>>>>> that OpenJPA was loaded in when the app is undeployed.
>>>>>
>>>>> -Patrick
>>>>>
>>>>> On 7/17/07, robert burrell donkin
>>>>> <ro...@gmail.com> wrote:
>>>>>> On 7/17/07, Kevan Miller <ke...@gmail.com> wrote:
>>>>>> >
>>>>>> > On Jul 17, 2007, at 12:15 AM, Craig L Russell wrote:
>>>>>> >
>>>>>> > >
>>>>>> > > On Jul 16, 2007, at 7:48 PM, Kevan Miller wrote:
>>>>>> > >
>>>>>> > >>
>>>>>> > >> On Jul 16, 2007, at 10:26 PM, Pinaki Poddar wrote:
>>>>>> > >>
>>>>>> > >>> Just to clarify:
>>>>>> > >>> I meant it should be the loader that loaded Person.class
>>>>>> where
>>>>>> > >>> Person is
>>>>>> > >>> the persistence capable class, *not*
>>>>>> > >>>
>>>>>> org.apache.openjpa.enhance.PersistenceCapable.class.getClassLoade
>>>>>> r
>>>>>> > >>> ().
>>>>>> > >>>
>>>>>> > >>> But testing in multi-classloader environment is required to
>>>>>> > >>> validate any
>>>>>> > >>> changes of this nature.
>>>>>> > >>
>>>>>> > >> Yes, that's how I interpreted your statement. My point is
>>>>>> that if
>>>>>> > >> SSN is a field of Person, the ClassLoader of SSN is not
>>>>>> > >> necessarily the ClassLoader for Person. I'd be concerned
>>>>>> that the
>>>>>> > >> Person ClassLoader can't load SSN. In that case, your
>>>>>> technique
>>>>>> > >> wouldn't work...
>>>>>> > >
>>>>>> > > Just to clarify, if SSN is the type of a field of Person, the
>>>>>> > > ClassLoader of Person.class must be able to load SSN
>>>>>> (either itself
>>>>>> > > or via a parent ClassLoader) or you will have a linkage
>>>>>> error while
>>>>>> > > loading Person.
>>>>>> >
>>>>>> > Craig,
>>>>>> > You are correct. The declared type must be loadable. I was
>>>>>> thinking
>>>>>> > of an SSNImpl type which need not be. However, that's not
>>>>>> really
>>>>>> > relevant to the problem at hand...
>>>>>> >
>>>>>> > Pinaki,
>>>>>> > I'm afraid that I may have improperly narrowed your
>>>>>> objectives by
>>>>>> > singling out fieldTypes. PCRegistry$Meta.pc and pcSuper
>>>>>> could also
>>>>>> > keep Classes/ClassLoaders alive...
>>>>>>
>>>>>> if there are several places where this may happen then
>>>>>> perhaps try a
>>>>>> variant on http://svn.apache.org/repos/asf/jakarta/commons/
>>>>>> proper/logging/trunk/src/java/org/apache/commons/logging/impl/
>>>>>> WeakHashtable.java
>>>>>>
>>>>>> - robert
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Patrick Linskey
>>>>> 202 669 5907
>>>>
>>>
>>
>
> Craig Russell
> Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
> 408 276-5638 mailto:Craig.Russell@sun.com
> P.S. A good JDO? O, Gasp!
>
Re: PCRegistry ClassLoader memory leak
Posted by Craig L Russell <Cr...@Sun.COM>.
On Jul 18, 2007, at 3:51 PM, Marc Prud'hommeaux wrote:
>
> On Jul 18, 2007, at 2:52 PM, Kevan Miller wrote:
>
>>
>> On Jul 17, 2007, at 5:32 PM, Marc Prud'hommeaux wrote:
>>
>>>
>>> Correct me if I am wrong, but I think the problem is essentially
>>> that we have a Map of SomeClass:Object->Object->...->SomeClass,
>>> and therefore SomeClass can never be garbage collected,
>>> regardless of whether the keys in the Map are soft or weak, and
>>> regardless of whether the class' ClassLoader is referenced
>>> elsewhere or not.
>>>
>>> Another possible solution might be to find every place where we
>>> have a Class field that could possibly be set to a
>>> PersistenceCapable, and turn them all into a
>>> WeakReference<Class>. I haven't checked to see how big a change
>>> that would be, but it might be one of the simpler solutions (and
>>> it might be faster than storing the class name as a String and
>>> doing a Class.forName() every time we need access to the Class
>>> object).
>>
>> There are two categories of references that are keeping
>> ClassLoaders alive.
>>
>> (1) References to classes (i.e. Meta fields pcClass, fieldTypes[],
>> and pcSuper). These can all be wrapped fairly easily using
>> WeakReferences. I've fixed these, locally.
>
> Sounds good.
>
>> (2) References to objects (i.e Meta.pc). This is a problem.
>> Meta.pc is the only reference to the PersistenceCapable object.
>> Since Meta.pc is the only reference to the PersistenceCapable
>> object, it can't be a WeakReference.
If the only time the WeakReference will be nullified is when the
class is no longer referenced, isn't it ok to use WeakReference? What
are the circumstances under which the class is not referenced? If its
class loader is garbage collected, or no class references the pc class?
The Registry need only keep track of currently used classes. If the
class loader decides to unload the class, it's ok to remove it from
the registry, as long as when the class loader needs it again it
initializes it.
It's been a long time since this architecture was put into place but
I don't recall any real issues like these...
Craig
>>
>> Any thoughts on fixing the problem with Meta.pc? Does it need to
>> be an object? Can we generate an instance of PersistenceCapable as
>> needed?
>
> That is unfortunate. To my knowledge, the only reason we need to
> keep around an instance of the object is that it is easy to
> construct new instances using PersistenceCapable.pcNewInstance()
> without having to use reflection each time or worry about security.
> I'm not sure of any way around this.
>
>> Or, on a different tack, can PCRegistry be made non-static?
>> Associated with the PU, for instance?
>
> I personally think it would be nice if we could get rid of
> PCRegistry being static. However, this part of the problem with the
> Meta.pc issue in the previous paragraph: the static initializer of
> any enhanced persistent class will do:
>
> PCRegistry.register(MyClass.class, pcFieldNames, pcFieldTypes,
> pcFieldFlags, pcPCSuperclass, "MyClass", new MyClass());
>
> If PCRegistry weren't static, then we wouldn't be able to do this.
>
> Also, I think there are a number of places where we rely on
> PCRegistry being static for other reasons. I think it would be nice
> to get rid of these, but that project might be too large a scope
> just to fix this bug.
>
>
>> As Patrick mentions, another approach is to load OpenJPA in every
>> application classloader (or a unique OpenJPA ClassLoader per
>> application ClassLoader). I really don't want to do this... I'd
>> prefer a private interface to notify you of when ClassLoaders are
>> no longer valid...
>>
>> --kevan
>>
>>
>>>
>>>
>>> On Jul 17, 2007, at 10:14 AM, Patrick Linskey wrote:
>>>
>>>> Of course, another approach is for Geronimo to trash the
>>>> classloader
>>>> that OpenJPA was loaded in when the app is undeployed.
>>>>
>>>> -Patrick
>>>>
>>>> On 7/17/07, robert burrell donkin
>>>> <ro...@gmail.com> wrote:
>>>>> On 7/17/07, Kevan Miller <ke...@gmail.com> wrote:
>>>>> >
>>>>> > On Jul 17, 2007, at 12:15 AM, Craig L Russell wrote:
>>>>> >
>>>>> > >
>>>>> > > On Jul 16, 2007, at 7:48 PM, Kevan Miller wrote:
>>>>> > >
>>>>> > >>
>>>>> > >> On Jul 16, 2007, at 10:26 PM, Pinaki Poddar wrote:
>>>>> > >>
>>>>> > >>> Just to clarify:
>>>>> > >>> I meant it should be the loader that loaded Person.class
>>>>> where
>>>>> > >>> Person is
>>>>> > >>> the persistence capable class, *not*
>>>>> > >>>
>>>>> org.apache.openjpa.enhance.PersistenceCapable.class.getClassLoader
>>>>> > >>> ().
>>>>> > >>>
>>>>> > >>> But testing in multi-classloader environment is required to
>>>>> > >>> validate any
>>>>> > >>> changes of this nature.
>>>>> > >>
>>>>> > >> Yes, that's how I interpreted your statement. My point is
>>>>> that if
>>>>> > >> SSN is a field of Person, the ClassLoader of SSN is not
>>>>> > >> necessarily the ClassLoader for Person. I'd be concerned
>>>>> that the
>>>>> > >> Person ClassLoader can't load SSN. In that case, your
>>>>> technique
>>>>> > >> wouldn't work...
>>>>> > >
>>>>> > > Just to clarify, if SSN is the type of a field of Person, the
>>>>> > > ClassLoader of Person.class must be able to load SSN
>>>>> (either itself
>>>>> > > or via a parent ClassLoader) or you will have a linkage
>>>>> error while
>>>>> > > loading Person.
>>>>> >
>>>>> > Craig,
>>>>> > You are correct. The declared type must be loadable. I was
>>>>> thinking
>>>>> > of an SSNImpl type which need not be. However, that's not really
>>>>> > relevant to the problem at hand...
>>>>> >
>>>>> > Pinaki,
>>>>> > I'm afraid that I may have improperly narrowed your
>>>>> objectives by
>>>>> > singling out fieldTypes. PCRegistry$Meta.pc and pcSuper could
>>>>> also
>>>>> > keep Classes/ClassLoaders alive...
>>>>>
>>>>> if there are several places where this may happen then perhaps
>>>>> try a
>>>>> variant on http://svn.apache.org/repos/asf/jakarta/commons/
>>>>> proper/logging/trunk/src/java/org/apache/commons/logging/impl/
>>>>> WeakHashtable.java
>>>>>
>>>>> - robert
>>>>>
>>>>
>>>>
>>>> --
>>>> Patrick Linskey
>>>> 202 669 5907
>>>
>>
>
Craig Russell
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:Craig.Russell@sun.com
P.S. A good JDO? O, Gasp!
Re: PCRegistry ClassLoader memory leak
Posted by Marc Prud'hommeaux <mp...@apache.org>.
On Jul 18, 2007, at 2:52 PM, Kevan Miller wrote:
>
> On Jul 17, 2007, at 5:32 PM, Marc Prud'hommeaux wrote:
>
>>
>> Correct me if I am wrong, but I think the problem is essentially
>> that we have a Map of SomeClass:Object->Object->...->SomeClass,
>> and therefore SomeClass can never be garbage collected, regardless
>> of whether the keys in the Map are soft or weak, and regardless of
>> whether the class' ClassLoader is referenced elsewhere or not.
>>
>> Another possible solution might be to find every place where we
>> have a Class field that could possibly be set to a
>> PersistenceCapable, and turn them all into a WeakReference<Class>.
>> I haven't checked to see how big a change that would be, but it
>> might be one of the simpler solutions (and it might be faster than
>> storing the class name as a String and doing a Class.forName()
>> every time we need access to the Class object).
>
> There are two categories of references that are keeping
> ClassLoaders alive.
>
> (1) References to classes (i.e. Meta fields pcClass, fieldTypes[],
> and pcSuper). These can all be wrapped fairly easily using
> WeakReferences. I've fixed these, locally.
Sounds good.
> (2) References to objects (i.e Meta.pc). This is a problem. Meta.pc
> is the only reference to the PersistenceCapable object. Since
> Meta.pc is the only reference to the PersistenceCapable object, it
> can't be a WeakReference.
>
> Any thoughts on fixing the problem with Meta.pc? Does it need to be
> an object? Can we generate an instance of PersistenceCapable as
> needed?
That is unfortunate. To my knowledge, the only reason we need to keep
around an instance of the object is that it is easy to construct new
instances using PersistenceCapable.pcNewInstance() without having to
use reflection each time or worry about security. I'm not sure of any
way around this.
> Or, on a different tack, can PCRegistry be made non-static?
> Associated with the PU, for instance?
I personally think it would be nice if we could get rid of PCRegistry
being static. However, this part of the problem with the Meta.pc
issue in the previous paragraph: the static initializer of any
enhanced persistent class will do:
PCRegistry.register(MyClass.class, pcFieldNames, pcFieldTypes,
pcFieldFlags, pcPCSuperclass, "MyClass", new MyClass());
If PCRegistry weren't static, then we wouldn't be able to do this.
Also, I think there are a number of places where we rely on
PCRegistry being static for other reasons. I think it would be nice
to get rid of these, but that project might be too large a scope just
to fix this bug.
> As Patrick mentions, another approach is to load OpenJPA in every
> application classloader (or a unique OpenJPA ClassLoader per
> application ClassLoader). I really don't want to do this... I'd
> prefer a private interface to notify you of when ClassLoaders are
> no longer valid...
>
> --kevan
>
>
>>
>>
>> On Jul 17, 2007, at 10:14 AM, Patrick Linskey wrote:
>>
>>> Of course, another approach is for Geronimo to trash the classloader
>>> that OpenJPA was loaded in when the app is undeployed.
>>>
>>> -Patrick
>>>
>>> On 7/17/07, robert burrell donkin <ro...@gmail.com>
>>> wrote:
>>>> On 7/17/07, Kevan Miller <ke...@gmail.com> wrote:
>>>> >
>>>> > On Jul 17, 2007, at 12:15 AM, Craig L Russell wrote:
>>>> >
>>>> > >
>>>> > > On Jul 16, 2007, at 7:48 PM, Kevan Miller wrote:
>>>> > >
>>>> > >>
>>>> > >> On Jul 16, 2007, at 10:26 PM, Pinaki Poddar wrote:
>>>> > >>
>>>> > >>> Just to clarify:
>>>> > >>> I meant it should be the loader that loaded Person.class
>>>> where
>>>> > >>> Person is
>>>> > >>> the persistence capable class, *not*
>>>> > >>>
>>>> org.apache.openjpa.enhance.PersistenceCapable.class.getClassLoader
>>>> > >>> ().
>>>> > >>>
>>>> > >>> But testing in multi-classloader environment is required to
>>>> > >>> validate any
>>>> > >>> changes of this nature.
>>>> > >>
>>>> > >> Yes, that's how I interpreted your statement. My point is
>>>> that if
>>>> > >> SSN is a field of Person, the ClassLoader of SSN is not
>>>> > >> necessarily the ClassLoader for Person. I'd be concerned
>>>> that the
>>>> > >> Person ClassLoader can't load SSN. In that case, your
>>>> technique
>>>> > >> wouldn't work...
>>>> > >
>>>> > > Just to clarify, if SSN is the type of a field of Person, the
>>>> > > ClassLoader of Person.class must be able to load SSN (either
>>>> itself
>>>> > > or via a parent ClassLoader) or you will have a linkage
>>>> error while
>>>> > > loading Person.
>>>> >
>>>> > Craig,
>>>> > You are correct. The declared type must be loadable. I was
>>>> thinking
>>>> > of an SSNImpl type which need not be. However, that's not really
>>>> > relevant to the problem at hand...
>>>> >
>>>> > Pinaki,
>>>> > I'm afraid that I may have improperly narrowed your objectives by
>>>> > singling out fieldTypes. PCRegistry$Meta.pc and pcSuper could
>>>> also
>>>> > keep Classes/ClassLoaders alive...
>>>>
>>>> if there are several places where this may happen then perhaps
>>>> try a
>>>> variant on http://svn.apache.org/repos/asf/jakarta/commons/
>>>> proper/logging/trunk/src/java/org/apache/commons/logging/impl/
>>>> WeakHashtable.java
>>>>
>>>> - robert
>>>>
>>>
>>>
>>> --
>>> Patrick Linskey
>>> 202 669 5907
>>
>
Re: PCRegistry ClassLoader memory leak
Posted by Kevan Miller <ke...@gmail.com>.
On Jul 17, 2007, at 5:32 PM, Marc Prud'hommeaux wrote:
>
> Correct me if I am wrong, but I think the problem is essentially
> that we have a Map of SomeClass:Object->Object->...->SomeClass, and
> therefore SomeClass can never be garbage collected, regardless of
> whether the keys in the Map are soft or weak, and regardless of
> whether the class' ClassLoader is referenced elsewhere or not.
>
> Another possible solution might be to find every place where we
> have a Class field that could possibly be set to a
> PersistenceCapable, and turn them all into a WeakReference<Class>.
> I haven't checked to see how big a change that would be, but it
> might be one of the simpler solutions (and it might be faster than
> storing the class name as a String and doing a Class.forName()
> every time we need access to the Class object).
There are two categories of references that are keeping ClassLoaders
alive.
(1) References to classes (i.e. Meta fields pcClass, fieldTypes[],
and pcSuper). These can all be wrapped fairly easily using
WeakReferences. I've fixed these, locally.
(2) References to objects (i.e Meta.pc). This is a problem. Meta.pc
is the only reference to the PersistenceCapable object. Since Meta.pc
is the only reference to the PersistenceCapable object, it can't be a
WeakReference.
Any thoughts on fixing the problem with Meta.pc? Does it need to be
an object? Can we generate an instance of PersistenceCapable as
needed? Or, on a different tack, can PCRegistry be made non-static?
Associated with the PU, for instance?
As Patrick mentions, another approach is to load OpenJPA in every
application classloader (or a unique OpenJPA ClassLoader per
application ClassLoader). I really don't want to do this... I'd
prefer a private interface to notify you of when ClassLoaders are no
longer valid...
--kevan
>
>
> On Jul 17, 2007, at 10:14 AM, Patrick Linskey wrote:
>
>> Of course, another approach is for Geronimo to trash the classloader
>> that OpenJPA was loaded in when the app is undeployed.
>>
>> -Patrick
>>
>> On 7/17/07, robert burrell donkin <ro...@gmail.com>
>> wrote:
>>> On 7/17/07, Kevan Miller <ke...@gmail.com> wrote:
>>> >
>>> > On Jul 17, 2007, at 12:15 AM, Craig L Russell wrote:
>>> >
>>> > >
>>> > > On Jul 16, 2007, at 7:48 PM, Kevan Miller wrote:
>>> > >
>>> > >>
>>> > >> On Jul 16, 2007, at 10:26 PM, Pinaki Poddar wrote:
>>> > >>
>>> > >>> Just to clarify:
>>> > >>> I meant it should be the loader that loaded Person.class where
>>> > >>> Person is
>>> > >>> the persistence capable class, *not*
>>> > >>>
>>> org.apache.openjpa.enhance.PersistenceCapable.class.getClassLoader
>>> > >>> ().
>>> > >>>
>>> > >>> But testing in multi-classloader environment is required to
>>> > >>> validate any
>>> > >>> changes of this nature.
>>> > >>
>>> > >> Yes, that's how I interpreted your statement. My point is
>>> that if
>>> > >> SSN is a field of Person, the ClassLoader of SSN is not
>>> > >> necessarily the ClassLoader for Person. I'd be concerned
>>> that the
>>> > >> Person ClassLoader can't load SSN. In that case, your technique
>>> > >> wouldn't work...
>>> > >
>>> > > Just to clarify, if SSN is the type of a field of Person, the
>>> > > ClassLoader of Person.class must be able to load SSN (either
>>> itself
>>> > > or via a parent ClassLoader) or you will have a linkage error
>>> while
>>> > > loading Person.
>>> >
>>> > Craig,
>>> > You are correct. The declared type must be loadable. I was
>>> thinking
>>> > of an SSNImpl type which need not be. However, that's not really
>>> > relevant to the problem at hand...
>>> >
>>> > Pinaki,
>>> > I'm afraid that I may have improperly narrowed your objectives by
>>> > singling out fieldTypes. PCRegistry$Meta.pc and pcSuper could also
>>> > keep Classes/ClassLoaders alive...
>>>
>>> if there are several places where this may happen then perhaps
>>> try a
>>> variant on http://svn.apache.org/repos/asf/jakarta/commons/proper/
>>> logging/trunk/src/java/org/apache/commons/logging/impl/
>>> WeakHashtable.java
>>>
>>> - robert
>>>
>>
>>
>> --
>> Patrick Linskey
>> 202 669 5907
>
Re: PCRegistry ClassLoader memory leak
Posted by Marc Prud'hommeaux <mp...@apache.org>.
Correct me if I am wrong, but I think the problem is essentially that
we have a Map of SomeClass:Object->Object->...->SomeClass, and
therefore SomeClass can never be garbage collected, regardless of
whether the keys in the Map are soft or weak, and regardless of
whether the class' ClassLoader is referenced elsewhere or not.
Another possible solution might be to find every place where we have
a Class field that could possibly be set to a PersistenceCapable, and
turn them all into a WeakReference<Class>. I haven't checked to see
how big a change that would be, but it might be one of the simpler
solutions (and it might be faster than storing the class name as a
String and doing a Class.forName() every time we need access to the
Class object).
On Jul 17, 2007, at 10:14 AM, Patrick Linskey wrote:
> Of course, another approach is for Geronimo to trash the classloader
> that OpenJPA was loaded in when the app is undeployed.
>
> -Patrick
>
> On 7/17/07, robert burrell donkin <ro...@gmail.com>
> wrote:
>> On 7/17/07, Kevan Miller <ke...@gmail.com> wrote:
>> >
>> > On Jul 17, 2007, at 12:15 AM, Craig L Russell wrote:
>> >
>> > >
>> > > On Jul 16, 2007, at 7:48 PM, Kevan Miller wrote:
>> > >
>> > >>
>> > >> On Jul 16, 2007, at 10:26 PM, Pinaki Poddar wrote:
>> > >>
>> > >>> Just to clarify:
>> > >>> I meant it should be the loader that loaded Person.class where
>> > >>> Person is
>> > >>> the persistence capable class, *not*
>> > >>>
>> org.apache.openjpa.enhance.PersistenceCapable.class.getClassLoader
>> > >>> ().
>> > >>>
>> > >>> But testing in multi-classloader environment is required to
>> > >>> validate any
>> > >>> changes of this nature.
>> > >>
>> > >> Yes, that's how I interpreted your statement. My point is
>> that if
>> > >> SSN is a field of Person, the ClassLoader of SSN is not
>> > >> necessarily the ClassLoader for Person. I'd be concerned that
>> the
>> > >> Person ClassLoader can't load SSN. In that case, your technique
>> > >> wouldn't work...
>> > >
>> > > Just to clarify, if SSN is the type of a field of Person, the
>> > > ClassLoader of Person.class must be able to load SSN (either
>> itself
>> > > or via a parent ClassLoader) or you will have a linkage error
>> while
>> > > loading Person.
>> >
>> > Craig,
>> > You are correct. The declared type must be loadable. I was thinking
>> > of an SSNImpl type which need not be. However, that's not really
>> > relevant to the problem at hand...
>> >
>> > Pinaki,
>> > I'm afraid that I may have improperly narrowed your objectives by
>> > singling out fieldTypes. PCRegistry$Meta.pc and pcSuper could also
>> > keep Classes/ClassLoaders alive...
>>
>> if there are several places where this may happen then perhaps try a
>> variant on http://svn.apache.org/repos/asf/jakarta/commons/proper/
>> logging/trunk/src/java/org/apache/commons/logging/impl/
>> WeakHashtable.java
>>
>> - robert
>>
>
>
> --
> Patrick Linskey
> 202 669 5907
Re: PCRegistry ClassLoader memory leak
Posted by Patrick Linskey <pl...@gmail.com>.
Of course, another approach is for Geronimo to trash the classloader
that OpenJPA was loaded in when the app is undeployed.
-Patrick
On 7/17/07, robert burrell donkin <ro...@gmail.com> wrote:
> On 7/17/07, Kevan Miller <ke...@gmail.com> wrote:
> >
> > On Jul 17, 2007, at 12:15 AM, Craig L Russell wrote:
> >
> > >
> > > On Jul 16, 2007, at 7:48 PM, Kevan Miller wrote:
> > >
> > >>
> > >> On Jul 16, 2007, at 10:26 PM, Pinaki Poddar wrote:
> > >>
> > >>> Just to clarify:
> > >>> I meant it should be the loader that loaded Person.class where
> > >>> Person is
> > >>> the persistence capable class, *not*
> > >>> org.apache.openjpa.enhance.PersistenceCapable.class.getClassLoader
> > >>> ().
> > >>>
> > >>> But testing in multi-classloader environment is required to
> > >>> validate any
> > >>> changes of this nature.
> > >>
> > >> Yes, that's how I interpreted your statement. My point is that if
> > >> SSN is a field of Person, the ClassLoader of SSN is not
> > >> necessarily the ClassLoader for Person. I'd be concerned that the
> > >> Person ClassLoader can't load SSN. In that case, your technique
> > >> wouldn't work...
> > >
> > > Just to clarify, if SSN is the type of a field of Person, the
> > > ClassLoader of Person.class must be able to load SSN (either itself
> > > or via a parent ClassLoader) or you will have a linkage error while
> > > loading Person.
> >
> > Craig,
> > You are correct. The declared type must be loadable. I was thinking
> > of an SSNImpl type which need not be. However, that's not really
> > relevant to the problem at hand...
> >
> > Pinaki,
> > I'm afraid that I may have improperly narrowed your objectives by
> > singling out fieldTypes. PCRegistry$Meta.pc and pcSuper could also
> > keep Classes/ClassLoaders alive...
>
> if there are several places where this may happen then perhaps try a
> variant on http://svn.apache.org/repos/asf/jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/WeakHashtable.java
>
> - robert
>
--
Patrick Linskey
202 669 5907
Re: PCRegistry ClassLoader memory leak
Posted by robert burrell donkin <ro...@gmail.com>.
On 7/17/07, Kevan Miller <ke...@gmail.com> wrote:
>
> On Jul 17, 2007, at 12:15 AM, Craig L Russell wrote:
>
> >
> > On Jul 16, 2007, at 7:48 PM, Kevan Miller wrote:
> >
> >>
> >> On Jul 16, 2007, at 10:26 PM, Pinaki Poddar wrote:
> >>
> >>> Just to clarify:
> >>> I meant it should be the loader that loaded Person.class where
> >>> Person is
> >>> the persistence capable class, *not*
> >>> org.apache.openjpa.enhance.PersistenceCapable.class.getClassLoader
> >>> ().
> >>>
> >>> But testing in multi-classloader environment is required to
> >>> validate any
> >>> changes of this nature.
> >>
> >> Yes, that's how I interpreted your statement. My point is that if
> >> SSN is a field of Person, the ClassLoader of SSN is not
> >> necessarily the ClassLoader for Person. I'd be concerned that the
> >> Person ClassLoader can't load SSN. In that case, your technique
> >> wouldn't work...
> >
> > Just to clarify, if SSN is the type of a field of Person, the
> > ClassLoader of Person.class must be able to load SSN (either itself
> > or via a parent ClassLoader) or you will have a linkage error while
> > loading Person.
>
> Craig,
> You are correct. The declared type must be loadable. I was thinking
> of an SSNImpl type which need not be. However, that's not really
> relevant to the problem at hand...
>
> Pinaki,
> I'm afraid that I may have improperly narrowed your objectives by
> singling out fieldTypes. PCRegistry$Meta.pc and pcSuper could also
> keep Classes/ClassLoaders alive...
if there are several places where this may happen then perhaps try a
variant on http://svn.apache.org/repos/asf/jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/WeakHashtable.java
- robert
RE: PCRegistry ClassLoader memory leak
Posted by Pinaki Poddar <pp...@bea.com>.
Kevan,
> I'm afraid that I may have improperly narrowed your objectives by
singling out fieldTypes. PCRegistry$Meta.pc and pcSuper > could also
keep Classes/ClassLoaders alive...
I am afraid that I have spelt your name wrong in JIRA-285...
The latest patch (JIRA-285.patch.2.txt) stringifies
PCRegistry$Meta.pcSuper also as per your observation.
PCRegistry$Meta.pc, however, was declared as PersistenceCapable.
Tried stringifying the user-defined persistence capble class name and
recreate an instance in PCRegistry when required. The problem with that
approach is instantiation may not succeed because OpenJPA allows user
defined pc classes *not* to declare a public no-arg constructor. 6 Tests
in OpenJPA failed due to that restriction.
I have currently left PCRegistry$Meta.pc as it was.
Pinaki Poddar
972.834.2865
-----Original Message-----
From: Kevan Miller [mailto:kevan.miller@gmail.com]
Sent: Tuesday, July 17, 2007 9:17 AM
To: dev@openjpa.apache.org
Subject: Re: PCRegistry ClassLoader memory leak
>
> On Jul 16, 2007, at 7:48 PM, Kevan Miller wrote:
>
Pinaki,
I'm afraid that I may have improperly narrowed your objectives by
singling out fieldTypes. PCRegistry$Meta.pc and pcSuper could also keep
Classes/ClassLoaders alive...
--kevan
Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.
Re: PCRegistry ClassLoader memory leak
Posted by Kevan Miller <ke...@gmail.com>.
On Jul 17, 2007, at 12:15 AM, Craig L Russell wrote:
>
> On Jul 16, 2007, at 7:48 PM, Kevan Miller wrote:
>
>>
>> On Jul 16, 2007, at 10:26 PM, Pinaki Poddar wrote:
>>
>>> Just to clarify:
>>> I meant it should be the loader that loaded Person.class where
>>> Person is
>>> the persistence capable class, *not*
>>> org.apache.openjpa.enhance.PersistenceCapable.class.getClassLoader
>>> ().
>>>
>>> But testing in multi-classloader environment is required to
>>> validate any
>>> changes of this nature.
>>
>> Yes, that's how I interpreted your statement. My point is that if
>> SSN is a field of Person, the ClassLoader of SSN is not
>> necessarily the ClassLoader for Person. I'd be concerned that the
>> Person ClassLoader can't load SSN. In that case, your technique
>> wouldn't work...
>
> Just to clarify, if SSN is the type of a field of Person, the
> ClassLoader of Person.class must be able to load SSN (either itself
> or via a parent ClassLoader) or you will have a linkage error while
> loading Person.
Craig,
You are correct. The declared type must be loadable. I was thinking
of an SSNImpl type which need not be. However, that's not really
relevant to the problem at hand...
Pinaki,
I'm afraid that I may have improperly narrowed your objectives by
singling out fieldTypes. PCRegistry$Meta.pc and pcSuper could also
keep Classes/ClassLoaders alive...
--kevan
Re: PCRegistry ClassLoader memory leak
Posted by Marina Vatkina <Ma...@Sun.COM>.
I'm wondering if the EMF.close() either called by the user or by the container,
when the application is unloaded/undeployed can trigger the cleanup?
thanks,
-marina
Craig L Russell wrote:
>
> On Jul 16, 2007, at 7:48 PM, Kevan Miller wrote:
>
>>
>> On Jul 16, 2007, at 10:26 PM, Pinaki Poddar wrote:
>>
>>> Just to clarify:
>>> I meant it should be the loader that loaded Person.class where
>>> Person is
>>> the persistence capable class, *not*
>>> org.apache.openjpa.enhance.PersistenceCapable.class.getClassLoader().
>>>
>>> But testing in multi-classloader environment is required to validate
>>> any
>>> changes of this nature.
>>
>>
>> Yes, that's how I interpreted your statement. My point is that if SSN
>> is a field of Person, the ClassLoader of SSN is not necessarily the
>> ClassLoader for Person. I'd be concerned that the Person ClassLoader
>> can't load SSN. In that case, your technique wouldn't work...
>
>
> Just to clarify, if SSN is the type of a field of Person, the
> ClassLoader of Person.class must be able to load SSN (either itself or
> via a parent ClassLoader) or you will have a linkage error while
> loading Person.
>
> Craig
>
>> Either way, would definitely require testing...
>>
>> --kevan
>>
>>
>>
>
> Craig Russell
> Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
> 408 276-5638 mailto:Craig.Russell@sun.com
> P.S. A good JDO? O, Gasp!
>
Re: PCRegistry ClassLoader memory leak
Posted by Craig L Russell <Cr...@Sun.COM>.
On Jul 16, 2007, at 7:48 PM, Kevan Miller wrote:
>
> On Jul 16, 2007, at 10:26 PM, Pinaki Poddar wrote:
>
>> Just to clarify:
>> I meant it should be the loader that loaded Person.class where
>> Person is
>> the persistence capable class, *not*
>> org.apache.openjpa.enhance.PersistenceCapable.class.getClassLoader().
>>
>> But testing in multi-classloader environment is required to
>> validate any
>> changes of this nature.
>
> Yes, that's how I interpreted your statement. My point is that if
> SSN is a field of Person, the ClassLoader of SSN is not necessarily
> the ClassLoader for Person. I'd be concerned that the Person
> ClassLoader can't load SSN. In that case, your technique wouldn't
> work...
Just to clarify, if SSN is the type of a field of Person, the
ClassLoader of Person.class must be able to load SSN (either itself
or via a parent ClassLoader) or you will have a linkage error while
loading Person.
Craig
> Either way, would definitely require testing...
>
> --kevan
>
>
>
Craig Russell
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:Craig.Russell@sun.com
P.S. A good JDO? O, Gasp!
Re: PCRegistry ClassLoader memory leak
Posted by Kevan Miller <ke...@gmail.com>.
On Jul 16, 2007, at 10:26 PM, Pinaki Poddar wrote:
> Just to clarify:
> I meant it should be the loader that loaded Person.class where
> Person is
> the persistence capable class, *not*
> org.apache.openjpa.enhance.PersistenceCapable.class.getClassLoader().
>
> But testing in multi-classloader environment is required to
> validate any
> changes of this nature.
Yes, that's how I interpreted your statement. My point is that if SSN
is a field of Person, the ClassLoader of SSN is not necessarily the
ClassLoader for Person. I'd be concerned that the Person ClassLoader
can't load SSN. In that case, your technique wouldn't work... Either
way, would definitely require testing...
--kevan
RE: PCRegistry ClassLoader memory leak
Posted by Pinaki Poddar <pp...@bea.com>.
Just to clarify:
I meant it should be the loader that loaded Person.class where Person is
the persistence capable class, *not*
org.apache.openjpa.enhance.PersistenceCapable.class.getClassLoader().
But testing in multi-classloader environment is required to validate any
changes of this nature.
Pinaki Poddar
972.834.2865
-----Original Message-----
From: Kevan Miller [mailto:kevan.miller@gmail.com]
Sent: Monday, July 16, 2007 9:22 PM
To: dev@openjpa.apache.org
Subject: Re: PCRegistry ClassLoader memory leak
On Jul 16, 2007, at 6:43 PM, Pinaki Poddar wrote:
> Meta.fieldTypes is declared as Class[].
> They are accessed via PCRegistry::getFieldTypes(Class pcClass).
>
> The suggestion of "Change fieldTypes to be Strings instead" *perhaps*
> meant to change Meta.fieldTypes to String[] (as field type names). And
> only during
> PCRegistry::getFieldTypes() 'lazily' converting the String[]s to
> Class[]es (by the same ClassLoader of the PersistenceCapable class?).
Right, you'll need a ClassLoader to load from. Defaulting the the CL of
the PersistenceCapable class might work (for Geronimo), but doesn't seem
right either...
--kevan
Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.
Re: PCRegistry ClassLoader memory leak
Posted by Kevan Miller <ke...@gmail.com>.
On Jul 16, 2007, at 6:43 PM, Pinaki Poddar wrote:
> Meta.fieldTypes is declared as Class[].
> They are accessed via PCRegistry::getFieldTypes(Class pcClass).
>
> The suggestion of "Change fieldTypes to be Strings instead" *perhaps*
> meant to change Meta.fieldTypes to String[] (as field type names). And
> only during
> PCRegistry::getFieldTypes() 'lazily' converting the String[]s to
> Class[]es (by the same ClassLoader of the PersistenceCapable class?).
Right, you'll need a ClassLoader to load from. Defaulting the the CL
of the PersistenceCapable class might work (for Geronimo), but
doesn't seem right either...
--kevan
RE: PCRegistry ClassLoader memory leak
Posted by Pinaki Poddar <pp...@bea.com>.
Meta.fieldTypes is declared as Class[].
They are accessed via PCRegistry::getFieldTypes(Class pcClass).
The suggestion of "Change fieldTypes to be Strings instead" *perhaps*
meant to change Meta.fieldTypes to String[] (as field type names). And
only during
PCRegistry::getFieldTypes() 'lazily' converting the String[]s to
Class[]es (by the same ClassLoader of the PersistenceCapable class?).
Pinaki Poddar
972.834.2865
-----Original Message-----
From: Marc Prud'hommeaux [mailto:mprudhomapache@gmail.com] On Behalf Of
Marc Prud'hommeaux
Sent: Monday, July 16, 2007 5:37 PM
To: dev@openjpa.apache.org
Subject: Re: PCRegistry ClassLoader memory leak
I'm a little unclear about what "Change fieldTypes to be Strings
instead" means. Do you mean the key in the map, or something?
One other solution would be to get rid of the Map and just use some sort
of weak Set, but lookups would need to iterate through the set every
time. I don't know what kind of performance hit this would incur.
The best solution would probably to just eliminate this (and all) non-
constant static fields. I remember trying very hard to do this some time
ago, but I kept hitting walls preventing the complete elimination. Maybe
smarter minds than me can figure it out, though.
On Jul 16, 2007, at 3:17 PM, robert burrell donkin wrote:
> On 7/16/07, Patrick Linskey <pl...@gmail.com> wrote:
>> > Anyone have any ideas?
>>
>> Change fieldTypes to be Strings instead, and dematerialize them as
>> needed.
>
> after spending hundreds of hours pursuing other solutions to a similar
> issue, i that this would be wise.
>
> <rant>
> sounds very similar to the infamous commons logging leak. after a
> *lot* of work, Brian Stansberry wrote a custom hashtable
> implementation which (after a lot of testing and rewriting) didn't
> seem to leak with modern JVMs. we used reference queues and periodic
> purging.
>
> IMO the real problem is that the J2EE specification lacks application
> life cycle events. i've heard of other leaks (eg when using
> reflection) which are hard or impossible to fix. of course, it doesn't
> help that the classloader specifications are broke too...
> </rant>
>
> - robert
Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.
Re: PCRegistry ClassLoader memory leak
Posted by Marc Prud'hommeaux <mp...@apache.org>.
I'm a little unclear about what "Change fieldTypes to be Strings
instead" means. Do you mean the key in the map, or something?
One other solution would be to get rid of the Map and just use some
sort of weak Set, but lookups would need to iterate through the set
every time. I don't know what kind of performance hit this would incur.
The best solution would probably to just eliminate this (and all) non-
constant static fields. I remember trying very hard to do this some
time ago, but I kept hitting walls preventing the complete
elimination. Maybe smarter minds than me can figure it out, though.
On Jul 16, 2007, at 3:17 PM, robert burrell donkin wrote:
> On 7/16/07, Patrick Linskey <pl...@gmail.com> wrote:
>> > Anyone have any ideas?
>>
>> Change fieldTypes to be Strings instead, and dematerialize them as
>> needed.
>
> after spending hundreds of hours pursuing other solutions to a similar
> issue, i that this would be wise.
>
> <rant>
> sounds very similar to the infamous commons logging leak. after a
> *lot* of work, Brian Stansberry wrote a custom hashtable
> implementation which (after a lot of testing and rewriting) didn't
> seem to leak with modern JVMs. we used reference queues and periodic
> purging.
>
> IMO the real problem is that the J2EE specification lacks application
> life cycle events. i've heard of other leaks (eg when using
> reflection) which are hard or impossible to fix. of course, it doesn't
> help that the classloader specifications are broke too...
> </rant>
>
> - robert
Re: PCRegistry ClassLoader memory leak
Posted by robert burrell donkin <ro...@gmail.com>.
On 7/16/07, Patrick Linskey <pl...@gmail.com> wrote:
> > Anyone have any ideas?
>
> Change fieldTypes to be Strings instead, and dematerialize them as needed.
after spending hundreds of hours pursuing other solutions to a similar
issue, i that this would be wise.
<rant>
sounds very similar to the infamous commons logging leak. after a
*lot* of work, Brian Stansberry wrote a custom hashtable
implementation which (after a lot of testing and rewriting) didn't
seem to leak with modern JVMs. we used reference queues and periodic
purging.
IMO the real problem is that the J2EE specification lacks application
life cycle events. i've heard of other leaks (eg when using
reflection) which are hard or impossible to fix. of course, it doesn't
help that the classloader specifications are broke too...
</rant>
- robert
Re: PCRegistry ClassLoader memory leak
Posted by Patrick Linskey <pl...@gmail.com>.
> Anyone have any ideas?
Change fieldTypes to be Strings instead, and dematerialize them as needed.
-Patrick
On 7/16/07, Marc Prud'hommeaux <mp...@apache.org> wrote:
> Kevan-
>
> How many deploy/undeploys does it take before it runs out of memory?
>
> I recall a long time back we had a similar problem (although the
> error was entrenched in the same functionality contained in the
> javax.jdo.spi.JDOImplHelper. I don't know if a solution was ever
> found. Short of figuring out some way to listen for the death of a
> ClassLoader and manually unregistering metadata when that happens, I
> can't think of any way to deal with this automatically.
>
> Anyone have any ideas?
>
>
>
> On Jul 16, 2007, at 2:31 PM, Kevan Miller wrote:
>
> > Geronimo is running out of PermGen space in some simple deploy/
> > undeploy scenarios involving OpenJPA. The cause of the problem
> > seems to be the _metas table in PCRegistry. _metas is a
> > ConcurrentReferenceHashMap with WEAK reference keys and HARD
> > reference values. The keys are the PersistenceCapable classes.
> > While the values are the metadata for these classes which are
> > maintained by the internal Meta class.
> >
> > The cause of the ClassLoader memory leak is simple -- if any of the
> > objects/classes held by the Meta class (e.g. fieldTypes) have also
> > been loaded by the same ClassLoader used to load the
> > PersistenceCapable class, the PersistenceCapable class (the weak
> > key) will never be GCed. The value of the HashMap entry will always
> > maintain a hard reference to the ClassLoader. Since the ClassLoader
> > will never be GC'ed, the the the pcClass Class object will never be
> > GC'able...
> >
> > The problem can be easily recreated using current Geronimo trunk
> > and the Geronimo Daytrader application.
> >
> > --kevan
>
>
--
Patrick Linskey
202 669 5907
Re: PCRegistry ClassLoader memory leak
Posted by Marc Prud'hommeaux <mp...@apache.org>.
Kevan-
How many deploy/undeploys does it take before it runs out of memory?
I recall a long time back we had a similar problem (although the
error was entrenched in the same functionality contained in the
javax.jdo.spi.JDOImplHelper. I don't know if a solution was ever
found. Short of figuring out some way to listen for the death of a
ClassLoader and manually unregistering metadata when that happens, I
can't think of any way to deal with this automatically.
Anyone have any ideas?
On Jul 16, 2007, at 2:31 PM, Kevan Miller wrote:
> Geronimo is running out of PermGen space in some simple deploy/
> undeploy scenarios involving OpenJPA. The cause of the problem
> seems to be the _metas table in PCRegistry. _metas is a
> ConcurrentReferenceHashMap with WEAK reference keys and HARD
> reference values. The keys are the PersistenceCapable classes.
> While the values are the metadata for these classes which are
> maintained by the internal Meta class.
>
> The cause of the ClassLoader memory leak is simple -- if any of the
> objects/classes held by the Meta class (e.g. fieldTypes) have also
> been loaded by the same ClassLoader used to load the
> PersistenceCapable class, the PersistenceCapable class (the weak
> key) will never be GCed. The value of the HashMap entry will always
> maintain a hard reference to the ClassLoader. Since the ClassLoader
> will never be GC'ed, the the the pcClass Class object will never be
> GC'able...
>
> The problem can be easily recreated using current Geronimo trunk
> and the Geronimo Daytrader application.
>
> --kevan