While trying to fix the JBoss AOP + AS tests with the new jboss-classpool project, I bumped into a problem involving ScopedClassPoolRepositoryImpl and DefaultClassPool.

Our factories were using DefaultClassPool to represent null class loaders so that bootstrap classes could be loaded without duplicates. This turned out to be a problem because we were seeing duplicates of JBoss normal classes. Take a look at this example:

When this ClassLoader is requested to load class org.jboss.jms.client.container.StateCreationAspect, it will delegate to DefaultDomain, which will in turn find the original BaseClassLoader as a candidate to load that class. That BaseClassLoader loads the class and the result is returned.

When this example is ported to ClassPool-level, we get a CNFException. The reason for this is that the BaseClassPool vfsfile:xxx/server/all/conf/jboss-service.xml has a parent that can find the resource that contains the requested class:

This is similar to the problem explained above, and makes this class pool capable of loading classes that other classpools should be responsible to load. As a result, we get duplicates of the same CtClass. Its FIXME tells me that this was already spotted before as a possible bug.

I worked around this issue by replacing the usage of the Default ClassPool by this Singleton class:

Equally to DefaultClassPool, it has the SystemPath appended, but its getClassLoader method returns the SystemClassLoader, which is the closest I can get to Bootstrap class loader (take a look at the Javadoc for Class<?>. getResource for more on this).

That didn't solve my problem entirely, because ScopedClassPoolRepositoryImpl has its own “default” classpool field:

Differently from what one may have thought, the ClassPoolRepository class (from jboss-classpool project) is not a subclass of ScopedClassPoolRepositoryImpl, given this class cannot be subclassed. This has been an issue for me before, because I always thought my code would be much cleaner if I could extend ScopedClassPoolRepositoryImpl instead of delegating to it.

So, what should be done in this regard? I'm opening this thread se we can find an elegant solution to my problem. Should ScopedClassPoolRepository be singleton? Should we use ScopedClassPoolFactory at all (the problem with not doing that is that we will have duplicate code in both classes)? I know that ScopedClassPool and auxiliary classes have been created to be used as the ClassPool solution to AS, but so many things have changed ever since that this solution is far from being complete, and that's why we need jboss-classpool now. So now, I'm not even sure what to do of that.

Regarding ClassPool.getClassLoader()'s implementation, I don't think it is a bug, even though we are not using this implementation at all (ScopedClassPool overwrites it, and so does the new SystemClassPool class). I just think that it is a way of doing things that works with simple standalone scenarios, but that doesnot fulfill our needs with AS.

Regarding ClassPool.getClassLoader()'s implementation, I don't think it is a bug, even though we are not using this implementation at all (ScopedClassPool overwrites it, and so does the new SystemClassPool class). I just think that it is a way of doing things that works with simple standalone scenarios, even though that doesnot fulfill our needs with AS.

On a side note, the class pool that is used by factories to represent null class loaders is configured on org.jboss.classpool.spi.AbstractClassPoolFActory. Currently it can be set by callling AbstractClassPoolFactory.setDefaultClassPool() and it can be retrieved by callling AbstractClassPoolFactory.getDefaultClassPool().

The problem is that, IMO, this nomenclature is not appropriate. Javassist uses default ClassPool to denote the classpools that is used by default on some operations. In my case, I'm configuring the class pool that is used to represent null class loaders.

Any ideas on how to name those methods? I thought on setNullClassPool or on setBootstrapClassPoolNotice that anyway we will have a gap with the SystemClassPool nomenclature, which is being used because this class pool is associated to the SystemClassLoader.

On a private talk with Kabir and Ales, we decided that the best thing to do for now is to copy those ScopedCP classes to a new module of jboss-classpool project and "fix" everything that is getting in the way with those.

As Kabir mentioned, those ScopedCP classes were created by us to be used inside AS. Now, they are outdated since they won't work in AS and we ended up needing ve a project only for that.

I'm calling that new module scoped-classpool. Once it is ready, we can decide whether it would be a good idea to use those changes as a patch for Javassist, or whether we wanna keep it the way it is.

I have only one observation to do regarding this. It will add a dependency from jboss-reflect towards scoped-classpool module. The way we are today, we have all references to the ScopedCP classes, and no reference at all to the subclasses in jboss-classpool project. Plus, we will also have to edit several references to ScopedCP's in JBoss AOP project as well.

I'm calling that new module scoped-classpool. Once it is ready, we can decide whether it would be a good idea to use those changes as a patch for Javassist, or whether we wanna keep it the way it is.

That new module is created. I removed everything that I thought is no longer being used, and replaced all references to javassist.scopedpool classes by references to the new org.jboss.classpool.scoped classes.

A summary of what has been done can be obtained by looking at the svn history log:

- Copied Scoped ClassPool classes from Javassist to a new scopedpool module.

- ScopedCPRepository is no longer singleton, it is an abstract class with a protected constructor instead. The default class pool can be defined as a parameter of the constructor.

Now I'm open for suggestions on what should be the next step: to write a patch for Javassist; to do an alpha release of the new version and update depending projects (JBoss AOP and JBoss Reflection, besides deployers-vfs tests); or to do more refactoring and improve something that doesn't look ok.

Now I'm open for suggestions on what should be the next step: to write a patch for Javassist; to do an alpha release of the new version and update depending projects (JBoss AOP and JBoss Reflection, besides deployers-vfs tests); or to do more refactoring and improve something that doesn't look ok.

I don't think we need to push this to javassist at this stage if that is what you mean.

Now I'm open for suggestions on what should be the next step: to write a patch for Javassist; to do an alpha release of the new version and update depending projects (JBoss AOP and JBoss Reflection, besides deployers-vfs tests); or to do more refactoring and improve something that doesn't look ok.

Whatever gets you back working on the Reflect+Javassist impl asap. ;-)

Check if there are no regressions with dependening projects and perhaps create a new Alpha release.

Like I said, we should try to get Reflect fully working on Javassist + Classpools asap,

Now I'm open for suggestions on what should be the next step: to write a patch for Javassist; to do an alpha release of the new version and update depending projects (JBoss AOP and JBoss Reflection, besides deployers-vfs tests); or to do more refactoring and improve something that doesn't look ok.

Whatever gets you back working on the Reflect+Javassist impl asap. ;-)

Check if there are no regressions with dependening projects and perhaps create a new Alpha release.

Like I said, we should try to get Reflect fully working on Javassist + Classpools asap,

In order to speed up things, Kabir is gonna help you with this -- you just tell him where you need him the most.

Ok! I'm doing the alpha release and then I'm updating the depending projects with it.

After that, I'm going to port the changes to JBoss AOP trunk, as we agreed on before.

IMO, this porting and updating things is work for one person only. Kabir could help me by doing the implementation of the generics features that we are going to need. Without that, we won't be able to to see the whole picture, and that's the main piece that we are missing right now.