openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kevan Miller <kevan.mil...@gmail.com>
Subject Re: PCRegistry ClassLoader memory leak
Date Wed, 18 Jul 2007 21:52:12 GMT

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 <robertburrelldonkin@gmail.com>  
>> wrote:
>>> On 7/17/07, Kevan Miller <kevan.miller@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
>


Mime
View raw message