1 Reply Latest reply on Aug 21, 2005 8:02 PM by Shane Bryzak

    Issue with AOPClassPool.getCached()

    Shane Bryzak Master

      I've just recently had to work around an issue I had with the getCached() method in org.jboss.aop.AOPClassPool. In my scenario, I am using JBoss AOP in an application that utilizes multiple classloaders to load different services, some of which contain identical classes. Here's an example to summarise:

      ClassLoader A first loads class org.test.MyClass
      ClassLoader B then also loads class org.test.MyClass.

      The issue that occurs is that when MyClass is loaded with classloader B, it has already been loaded by classloader A, and the getCached() method will not find a locally cached MyClass for classloader B, and iterates through the other registered classloaders/pools to attempt to locate the class. This results in a CtClass instance being returned from classloader A's class pool instead of classloader B's.

      The fix (I believe) for this solution is to check whether the local classloader is capable of loading the specified classname, and if it is then it shouldn't search the other registered classloaders. Here's a patched version of getCached() that does this:

       protected CtClass getCached(String classname)
       {
       CtClass clazz = getCachedLocally(classname);
       if (clazz == null)
       {
       boolean isLocal = false;
      
       if (dcl != null)
       {
       // Determine the resource name of the (outer) class
       String classResourceName = (classname.indexOf('$') == -1 ?
       classname :
       classname.substring(0, classname.indexOf('$'))).
       replaceAll("[\\.]", "/") + ".class";
       // Can the class resource be loaded by the local classloader?
       isLocal = dcl.getResource(classResourceName) != null;
       }
      
       // If the class can be loaded with the local classloader, don't try
       // to load it with another registered classloader.
       if (!isLocal)
       {
       Map registeredCLs = manager.getRegisteredCLs();
       synchronized (registeredCLs)
       {
       Iterator it = registeredCLs.values().iterator();
       while (it.hasNext())
       {
       AOPClassPool pool = (AOPClassPool) it.next();
       if (pool.isUnloadedClassLoader())
       {
       AspectManager.instance().unregisterClassLoader(pool.dcl);
       continue;
       }
      
       clazz = pool.getCachedLocally(classname);
       if (clazz != null)
       {
       return clazz;
       }
       }
       }
      
       }
       }
       // *NOTE* NEED TO TEST WHEN SUPERCLASS IS IN ANOTHER UCL!!!!!!
       return clazz;
       }
      


      I'd be interested in hearing other people's thoughts on this issue. The solution I've proposed does not feel very elegant but I'm not yet familiar enough with the JBoss AOP source to suggest anything better.