[adrian.brock]

Finally, the testsuite test for intercepting an ejb gives duplicates, it looks like references are being kept.

org.jboss.test.aop.simpleejb.Simple is a remote ejb interface.

I don't know what a SoftCache is?

org.jboss.mx.loading.UnifiedClassLoader3@31988659
!--- interface org.jboss.test.aop.simpleejb.Simple
!--- !--- java.util.HashMap$Entry@1547458
!--- !--- !--- [Ljava.util.HashMap$Entry;@23838842
!--- !--- !--- !--- java.util.HashMap@1260960
!---!---!---!---!--- FieldReference private java.util.Map sun.misc.SoftCache.hash=sun.misc.SoftCache@10765342 Detail

This looks like it comes from the server module (SessionMetaData) or remoting?

See the other thread where we discussing other memory leaks
late last month.

The SoftCache is a Sun class used in object serialization.
It is a bit of a misnomer. Since it is not soft in my experience.
It holds references to the last N classes that were serialized.

If you do some other serialization on different classes,
this cache will drop these references.

[clebert.suconic]

"Adrian" wrote:

If you do some other serialization on different classes,
this cache will drop these references.

Yes, this is correct, and JVMTIInterface.forceReleaseOnSoftReferences() is calling a writeObject right after forced OutOfMemoryException, trying to release these SoftCaches on Serialization.

[clebert.suconic]

"Kabir" wrote:

AOP remoting has a lot of maps hanging onto methods

I have looked into that stuff.
From what I read from the code, I think would be easier to use PersistentReferences here (the package I created for JBossSerialization).

Oh well... if you start using that, we will have to move it to commons.
Or you could just use it from JbossSerialization, however I don't think that should be the right decision. You would be using an internal package from JBossSerialization to something not related to serialization. (reflection caching)

[clebert.suconic]

"Clebert" wrote:

Ah....

I guess this is ThreadPool/Remoting not clearing Thread.currentClassLoader before the thread goes back to the pool.

Lets wait on Tom on what he says. I guess we will need to open a JIRA for MemoryLeaks on remoting, due to ClassLoader stored on ThreadPool.

Just realized you were talking about AOPRemote before. This doesn't have anything to do with Tom Elrod. My bad.

But anyway, take a look if you don't have ClassLoaders into ThreadLocals.

[kabirkhan]

For completeness here are the VersionedObject strong references

org.jboss.mx.loading.UnifiedClassLoader3@3519976
!--- class org.jboss.test.aop.bean.Address
!--- !--- org.jboss.test.aop.bean.Address@10967412
!--- !--- !--- org.jboss.test.aop.bean.Person@32425470
!--- !--- !--- !--- java.util.Hashtable$Entry@27526938
!--- !--- !--- !--- !--- [Ljava.util.Hashtable$Entry;@8845383
!--- !--- !--- !--- !--- !--- java.util.Hashtable@2775646
!---!---!---!---!---!---!--- FieldReference protected java.util.Hashtable org.jboss.aspects.versioned.LocalSynchronizationManager.objectTable=org.jboss.aspects.versioned.LocalSynchronizationManager@32908873 Detail

org.jboss.mx.loading.UnifiedClassLoader3@3519976
!--- class org.jboss.test.aop.bean.Address
!--- !--- org.jboss.test.aop.bean.Address@10967412
!--- !--- !--- org.jboss.test.aop.bean.Person@32425470
!---!---!---!--- FieldReference java.lang.Object java.util.Hashtable$Entry.value=java.util.Hashtable$Entry@27526938 Detail
!---!---!---!--- FieldReference protected transient org.jboss.aop.Advised org.jboss.aspects.versioned.DistributedPOJOState.advised=org.jboss.aspects.versioned.DistributedPOJOState@30295560 Detail
!---!---!---!--- FieldReference private java.lang.Object java.lang.ref.Reference.referent=java.lang.ref.WeakReference@22541220 Detail

org.jboss.mx.loading.UnifiedClassLoader3@3519976
!--- class org.jboss.test.aop.bean.Address
!--- !--- org.jboss.test.aop.bean.Address@10967412
!--- !--- !--- java.util.Hashtable$Entry@25126806
!--- !--- !--- !--- [Ljava.util.Hashtable$Entry;@8845383
!--- !--- !--- !--- !--- java.util.Hashtable@2775646
!---!---!---!---!---!--- FieldReference protected java.util.Hashtable org.jboss.aspects.versioned.LocalSynchronizationManager.objectTable=org.jboss.aspects.versioned.LocalSynchronizationManager@32908873 Detail

org.jboss.mx.loading.UnifiedClassLoader3@3519976
!--- class org.jboss.test.aop.bean.Address
!--- !--- org.jboss.test.aop.bean.Address@10967412
!---!---!--- FieldReference private org.jboss.test.aop.bean.Address org.jboss.test.aop.bean.Person.address=org.jboss.test.aop.bean.Person@32425470 Detail
!---!---!--- FieldReference protected transient org.jboss.aop.Advised org.jboss.aspects.versioned.DistributedPOJOState.advised=org.jboss.aspects.versioned.DistributedPOJOState@27109920 Detail
!---!---!--- FieldReference private java.lang.Object java.lang.ref.Reference.referent=java.lang.ref.WeakReference@800877 Detail
!---!---!--- FieldReference private transient org.jboss.aop.InstanceAdvised org.jboss.aspects.versioned.VersionReference.advised=org.jboss.aspects.versioned.VersionReference@22782447 Detail
!---!---!--- FieldReference java.lang.Object java.util.Hashtable$Entry.value=java.util.Hashtable$Entry@25126806 Detail

[kabirkhan]

I got the last of the leaks cleaned up, so the injboss testsuite runs fine on the branch now. This is ignoring the SoftCache/ThreadLocal issues for the simple ejb test.

For the methodmaps held by the ClassProxyFactory (the stuff that made the remote tests fail), I used the PerstentmethodReference. I have copied the required classes to org.jboss.aop.util.reference for now.

[clebert.suconic]

I guess it would be better if we created a common package for reference since we both will be using the same package?

Any ideas?


I guess I will wait Scott/Adrian to comment on that.

[clebert.suconic]

We need to find those ThreadLocal references. At least to know where they are generated. Maybe use WeakReferences on them.

[adrian.brock]

I guess it would be better if we created a common package for reference since we both will be using the same package?

I've already asked you twice to look at the ClassInfo abstraction.

There is also an outstanding task for AOP to use the Javassist version,
although the issue there is for that implementation to understanding
classloading without actually having to load the class.

[kabirkhan]

I've already asked you twice to look at the ClassInfo abstraction.

I'm not sure if this is meant for me or Clebert? In any case, these fixes are for JBoss 4.0.4 GA (AOP 1.3.7). I see the ClassInfo/AOP integration as scheduled for AOP 2.0? In any case this will not be possible before JBoss 4.0.4 GA

[clebert.suconic]

"Adrian" wrote:
I've already asked you twice to look at the ClassInfo abstraction.

What does this has to do JBossSerialization my metadata stuff?

This is about using WeakReference or SoftReferences for reflection. This package I created to encapsulate recreation of reflection objects.

I mean creationg because every time you do:

Method method = someClass.getMethod(...);

You always get a new object. (a clone from an internal reflection object inside the Class).


And besides, I will be looking into what you mentioned:

http://jira.jboss.org/jira/browse/JBSER-48

[adrian.brock]

"Adrian" wrote:
I've already asked you twice to look at the ClassInfo abstraction.

What does this has to do JBossSerialization my metadata stuff?

This is about using WeakReference or SoftReferences for reflection. This package I created to encapsulate recreation of reflection objects.

I mean creationg because every time you do:

Method method = someClass.getMethod(...);

You always get a new object. (a clone from an internal reflection object inside the Class).


And besides, I will be looking into what you mentioned:

http://jira.jboss.org/jira/browse/JBSER-48

Every JEMS project has its own reflection abstraction/caching to workaround
the JDK deficienies.

This is both redundant and inefficient.

Short of us writing our own JDK, we need to work on one solution!

[clebert.suconic]

Well,

I don't know if these are JDK inefficiencies.

- It's just a fact that if you use reflection objects, you should use SoftReferences on them.
- And another fact is that WeakHashMap<ClassLoader,Class> will always leak. (this won't never release the ClassLoader).

Even I when using ClassInfo on JBossSerialization will probably have to extent it to still use Reflection Fields (if I remember correctly there isn't a way yet I could acces private and final fields). On that case I will still need to use PersistentReferences.

My discussion here with Kabir was when using reflection he shouldn't use strong references. On that case for a reflection object he will have to use SoftReference, and I suggested to use my package instead of re-testing for reference.get()==null.

And if we are both using the same package where should the package exist?

IMO It doesn't seem possible at this point simply drop the use of reflection in favor of javassist.

[adrian.brock]

Come on, we've discussed all this before.
Just because it requires a little bit of work for us all to use the
same thing, is not an excuse, you went ahead anyway
and continue to put barriers to adoption in place that are not really there.

Summary of what has gone before answering your questions:

- It's just a fact that if you use reflection objects, you should use SoftReferences on them.
- And another fact is that WeakHashMap<ClassLoader,Class> will always leak. (this won't never release the ClassLoader).

That is what ClassInfo does.

WeakHashMap<ClassLoader, SoftReference<ClassInfo>>

I think I'd actually also prefer using something like the Hibernate class
where you would have a MRU cache with the no much
referenced classes getting dropped and regenerated when required.

Even I when using ClassInfo on JBossSerialization will probably have to extent it to still use Reflection Fields (if I remember correctly there isn't a way yet I could acces private and final fields). On that case I will still need to use PersistentReferences.

I already told you, you could change it. It simply needs a setAccessible
for the reflection implementation.

And if we are both using the same package where should the package exist?

It will be standalone once we've sorted jboss-common as a
standalone project.

IMO It doesn't seem possible at this point simply drop the use of reflection in favor of javassist.

AOP needs to use javassist, because it has to look at the joinpoints
before doing the classloading. i.e. It has to analyse the class
before doing some weaving.
It is then only a short step to have javassist generate the more efficient
version of the reflection object using something like I already
demonstrated.

[clebert.suconic]

I'm not arguing to not use ClassInfo. I will look into that. I just didn't have enough time to do it.

The only thing here is that PersistentReference doesn't have anything to do with the MetaData.

I created PersistentReference to avoid exact a SoftReference on the value of WeakHashMap.

Let me explain.

IM*H*O, the correct way of your WeakHashMap should be:

WeakhashMap<ClassLoader, ClassInfo>

And never use a strong reference to Class/ClassLoader inside ClassInfo.


And, if you have anything with Class/Classloader as an indirect reference and it's an unique object, than you use SoftReference to cache it.

This way you don't need to recreate the whole ClassInfo in case of release.

PersistentReferences are just encapsulaing this behavior, that you really need if you don't want to keep the equivalent to ClassInfo in a SoftReference:

if (reference.get()==null)
{
 synchronized (this)
 {
 Reflectionobject|ClassInfoObject|javassistObject = whatever you do to recreate it.
 reference = new SoftReference(createdObject);
 }
}

So, I mean that when you start using SoftReference to any reflection like objects, you will need PersistentReference, unless you want to replicate this check everywhere.