Reflect BeanInfo cache change breaks Kernel

[alesj]

Looks like our change
- http://www.jboss.com/index.html?module=bb&op=viewtopic&t=145938&start=20
to Reflect's BeanInfo caching is the cause of this:
- http://lists.jboss.org/pipermail/jboss-development/2008-November/013128.html

Since when I change this in Kernel, to use 2.0.1.GA,
DeploymentClassLoaderTestCase test is failing:

testDeploymentClassLoader(org.jboss.test.kernel.deployment.test.DeploymentClassLoaderTestCase) Time elapsed: 0.031 sec <<< FAILURE!
junit.framework.AssertionFailedError: expected:<org.jboss.test.classloading.vfs.VFSClassLoader@1aaf194> but was:<org.jboss.test.classloading.vfs.VFSClassLoader@e3990b>
 at junit.framework.Assert.fail(Assert.java:47)
 at junit.framework.Assert.failNotEquals(Assert.java:277)
 at junit.framework.Assert.assertEquals(Assert.java:64)
 at junit.framework.Assert.assertEquals(Assert.java:71)
 at org.jboss.test.kernel.deployment.test.DeploymentClassLoaderTestCase.testDeploymentClassLoader(DeploymentClassLoaderTestCase.java:59)

Looks like ClassInfo is not 'immune' to different classloaders.
Should I put back CL as additional key?

[alesj]

"alesj" wrote:

Looks like ClassInfo is not 'immune' to different classloaders.
Should I put back CL as additional key?

Putting CL back as key helps.

I'll do another release + add some tests for this CL issue.

[kabirkhan]

Can you let me know when you have commited this? I'd like to run the aop tests in AS trunk with this just to make double sure

[adrian.brock]

"alesj" wrote:

Looks like ClassInfo is not 'immune' to different classloaders.
Should I put back CL as additional key?

The issue is that the ClassInfo is not usable as a key to a map since it only
checks the class name in the equals.
i.e. it has no understanding of classloader.

[alesj]

Can you let me know when you have commited this? I'd like to run the aop tests in AS trunk with this just to make double sure

I've already deployed 2.2.0-SNAPSHOT, if you wanna test it.

[alesj]

The issue is that the ClassInfo is not usable as a key to a map since it only
checks the class name in the equals.
i.e. it has no understanding of classloader.

I would expect this to fail:

 public void testClassLoaderCaching() throws Throwable
 {
 String className = BeanInfoEmpty.class.getName();
 Class<?> clazz = Class.forName(className);
 URL url1 = clazz.getProtectionDomain().getCodeSource().getLocation();
 URL[] urls = {url1};
 ClassLoader cl1 = new URLClassLoader(urls);

 className = ClassInfo.class.getName();
 clazz = Class.forName(className);
 URL url2 = clazz.getProtectionDomain().getCodeSource().getLocation();
 urls = new URL[]{url1, url2};
 ClassLoader cl2 = new URLClassLoader(urls);

 Configuration configuration = getConfiguration();

 ClassInfo ci1 = configuration.getClassInfo(className, cl1);
 ClassInfo ci2 = configuration.getClassInfo(className, cl2);
 assertSame(ci1, ci2);
 assertEquals(ci1.hashCode(), ci2.hashCode());

 className = "org.jboss.test.beaninfo.support.BeanInfoCache";
 BeanInfo bi1 = configuration.getBeanInfo(className, cl1);
 BeanInfo bi2 = configuration.getBeanInfo(className, cl2);
 assertFalse(bi1.equals(bi2)); // !-- HERE
 }

But BeanInfoCache's CL is always sun.misc.Launcher$AppClassLoader@a39137.
Although I would expect my URLCL.

Where am I wrong? :-)

[alesj]

Missing detail:

package org.jboss.test.beaninfo.support;

/**
 * BeanInfoEmpty.
 *
 * @author <a href="[e-mail]">Adrian Brock</a>
 * @version $Revision: 1.1 $
 */
public class BeanInfoEmpty
{
}

Same package as BeanInfoCache.

[kabirkhan]

The aop tests in AS trunk run fine with this snapshot

[alesj]

The issue is that the ClassInfo is not usable as a key to a map since it only
checks the class name in the equals.
i.e. it has no understanding of classloader.

How is this then different from what we already had?
CL + String (ClassInfo::getName) vs. CL + ClassInfo

But it must be different since the tests I added (similar to yours printCollection)
are now passing, where before they didn't. :-)

[alesj]

"alesj" wrote:
Where am I wrong?

Got it. ;-)
URLCL::parent must be explicitly set to null,
else it uses bootstrap/system CL as parent.

 String className = BeanInfoEmpty.class.getName();
 Class<?> clazz = Class.forName(className);
 URL url1 = clazz.getProtectionDomain().getCodeSource().getLocation();
 URL[] urls = {url1};
 ClassLoader cl1 = new URLClassLoader(urls, null);

[adrian.brock]

"alesj" wrote:

ClassLoader cl1 = new URLClassLoader(urls);

But BeanInfoCache's CL is always sun.misc.Launcher$AppClassLoader@a39137.
Although I would expect my URLCL.

Where am I wrong? :-)

You need a classloader that overrides loadClass() to not look at the parent.
The classloader you've constructed will just use the ClassLoader.getSystemClassLoader()
as the parent. i.e. the classpath

[alesj]

"alesj" wrote:

But it must be different since the tests I added (similar to yours printCollection)
are now passing, where before they didn't. :-)

It's different due to ClassInfoImpl::equals != DelegateClassInfo::equals.