1 2 3 Previous Next 31 Replies Latest reply on Sep 20, 2007 8:24 AM by Adrian Brock

    AOP asintegration WITHOUT the integration :-)

    Adrian Brock Master

      Related to these threads:

      New classloader tests - http://www.jboss.com/index.html?module=bb&op=viewtopic&t=114427
      Refactoring ASIntegration for new classloader -
      http://www.jboss.com/index.html?module=bb&op=viewtopic&t=112771
      AOP Scoping is WRONG!
      http://www.jboss.com/index.html?module=bb&op=viewtopic&t=112736

      I think I've found a way to make this work without having to any integration.
      The basic problem is the one I found with the TCL usage.
      When this is fixed (and some other things), we don't need any integration code.

      The new tests I wrote now all work with the following two changes:

      1) In JoinPointGenerator I now always use the classloader of the pool
      where the advice lives, not the TCL.
      2) I've added a new option "pushClassLoader" to the AspectManager
      which says that when generating advices it should be done under the
      classloader of the **Advised** class

      Obviously these changes are incomplete:
      a) Part (2) above should be using the classloader of the **Advice** class
      b) I'm only doing (2) for PER_VM and PER_CLASS since that's all I've tested so far
      c) There are other places besides the Advisor that generate advices
      d) There are a number of other places that use the TCL that need fixing
      in a similar way.

      There's one other fix that needs doing which is in (I haven't made this change)
      AspectManager.findClassPool()

      The use of Translatable is obsolete. It should really be doing something like:

      if (cl == null)
       cl = ClassLoader.getSystemClassLoader(); // Using SecurityActions;
      return registerClassLoader(cl);
      


      For now, I've made RealClassLoader (which is what both the new and old
      classloaders extend) implement Translatable.

        • 1. Re: AOP asintegration WITHOUT the integration :-)
          Adrian Brock Master

          So, once this change (use the correct classloader instead of guessing the TCL)
          is done throughout AOP there will be no need for ScopedClassLoaderDomains
          or any other integration code beyond the javassist ScopedClassPool.

          With just the few changes I've done already, all the tests I've written pass
          including the ones that have unexported classes and multiple versions
          of the classes which are some of the more complicated OSGi use cases.

          There's one caveat which is that to make the multiple version test work,
          I had to make the interceptor PER_CLASS scope.

          The notion of PER_VM needs redefining such that it takes into account
          multiple versions of the advice in different classloaders, but it should not have to
          do complicated things like the ScopedClassLoaderDomain to do that.

          The PER_VM advices should just be indexed by the classloader of the advice class.

          • 2. Re: AOP asintegration WITHOUT the integration :-)
            Adrian Brock Master

            I've reverted the pushClassLoader change.

            This was only working because the two different versions of the classes had the
            same -aop.xml

            I've updated the tests to show what the real problem is,
            i.e. the pointcut matching doesn't understand the classloading.

            • 3. Re: AOP asintegration WITHOUT the integration :-)
              Kabir Khan Master

               

              "adrian@jboss.org" wrote:
              I've reverted the pushClassLoader change.
              I've updated the tests to show what the real problem is,
              i.e. the pointcut matching doesn't understand the classloading.


              I've yet to completely understand your tests :-), but the point of the domain was to have different pointcuts/bindings deployed at different levels

              • 4. Re: AOP asintegration WITHOUT the integration :-)
                Kabir Khan Master

                Could you point me towards some information on which way the imports/exports work? For example in the following, my interpretation is that the "A" classloader imports/uses all classes from the underlying domain, apart from if they belong to the "a" package?

                public class SimpleImportAllUnitTestCase extends AOPIntegrationTest
                {
                 ...
                 /*
                 * A simple test that loads a class from another classloader
                 * that uses aop enhanced classes from our classloader.
                 *
                 * The other classloader importsAll from the classloading system.
                 */
                 public void testImportAll() throws Exception
                 {
                 ClassLoader classLoader = createClassLoader("A", true, PACKAGE_A);
                 try
                 {
                 Class<?> classA = classLoader.loadClass(CLASS_A);
                 classA.newInstance();
                 }
                 finally
                 {
                 unregisterClassLoader(classLoader);
                 }
                 }
                }
                
                


                • 5. Re: AOP asintegration WITHOUT the integration :-)
                  Adrian Brock Master

                  The mock classloader isn't documented anywhere AFAIR,
                  I only created it for testing purposes.

                  The importAll and exportAll (not in the MockClassLoader)
                  are meant to duplicate what the old UCL does.
                  In the example you quote, it does do "importAll" so it will load from the whole
                  domain that the classloader belongs to.

                  The mock classloader has two important properties.

                  setPackages() - what is exported
                  setPaths() - what is part of the classloader (but not necessarily exported to other classloaders)

                  There is a convience method to do both, setPackagesAndPaths()

                  In the api you show, the "PACKAGE_A" is what is included in the classloader and
                  what is exported.

                  The "true" means it will "importAll", i.e. look at all other classloaders in the
                  order they were registered to find the class before looking at its own
                  private (non-exported) classes.

                  Try adding enableTrace("org.jboss.classloader") to the test
                  and you will see in detail where it is trying to load classes from
                  and where it found it.

                  • 6. Re: AOP asintegration WITHOUT the integration :-)
                    Kabir Khan Master

                    That certainly clears things up :-) In ComplexImportAdviceUnitTestCase, could you please explain the purpose of the

                     MockClassLoaderPolicy supportPolicy = MockClassLoaderHelper.createMockClassLoaderPolicy("Support");
                     supportPolicy.setPathsAndPackageNames(PACKAGE_SUPPORT);
                     ClassLoader support = createClassLoader(supportPolicy);
                     try
                     {
                     MockClassLoaderPolicy bPolicy = MockClassLoaderHelper.createMockClassLoaderPolicy("B");
                     bPolicy.setPathsAndPackageNames(PACKAGE_B);
                     bPolicy.setDelegates(createDelegates(supportPolicy));
                     ClassLoader b = createClassLoader(bPolicy);
                    

                    I am thinking of the following line:
                    bPolicy.setDelegates(createDelegates(supportPolicy));
                    


                    • 7. Re: AOP asintegration WITHOUT the integration :-)
                      Kabir Khan Master

                      I've now had a look at the new aop classloading tests, and see the problem you have been describing.

                      So, the problem we have with the failing test is that it is trying to create the interceptor twice. This is what we solved before with the ScopedJBossClassPool/ScopedClassLoaderDomain. To summarize what you have probably said already with my own words, with the new classloaders, we don't really have scoped classloaders in this case. So the check to see if the classloader is scoped to create a new domain does not really work here... I see what you have done, which is basically to make sure that the correct classloader is used, however we are not really limiting what is available.

                      What is needed is maybe ***something along the lines of*** setting the classloader on the advicebindings (and other things) + a AdviceBinding.isVisble(ClassLoader) type method or something like that. I'll look more into this in the morning and hopefully come up with some better ideas :-)

                      • 8. Re: AOP asintegration WITHOUT the integration :-)
                        Adrian Brock Master

                         

                        "kabir.khan@jboss.com" wrote:

                        I am thinking of the following line:
                        bPolicy.setDelegates(createDelegates(supportPolicy));
                        


                        This says the classloader "b" imports the classloader "support".
                        i.e. all the exported classes from "support" are visible to "b".

                        • 9. Re: AOP asintegration WITHOUT the integration :-)
                          Kabir Khan Master

                          With the multiple versions test-case, we are basically deploying two similar jboss-aop.xml files into the system. One goes into A1, the other into A2

                           |---- Support1 -- A1
                          MOCK-
                           |---- Support2 -- A2
                          


                          I am not clear on what the visibility rules for resources are between the classloaders? Currently everything gets applied everywhere (in other words both in A1 and A2 we will get intercepted by TestInterceptor twice)
                          while it feels more natural that the jboss-aop.xml that is deployed into A1 only applies there, while the one that is depoyed into A2 only applies there. This is what the domains made easy :-) Of course we had the opposite case using the global domain where everything is deployed/visible everywhere. I guess I am trying to define this "partial" visiblity somehow wrt to the -aop.xml information.

                          Or should I not really care about this unless there is a "real" scope, i.e child domains, and just make sure that the AspectDefinitions have some indexing by classloader like you suggest?





                          • 10. Re: AOP asintegration WITHOUT the integration :-)
                            Kabir Khan Master

                             

                            "kabir.khan@jboss.com" wrote:

                            Or should I not really care about this unless there is a "real" scope, i.e child domains, and just make sure that the AspectDefinitions have some indexing by classloader like you suggest?


                            In other words, with the tests set up the way they are is it fine for the classes in A1 and A2 to get intercepted twice, just making sure that the the correct versions of the aspects are being used?

                            • 11. Re: AOP asintegration WITHOUT the integration :-)
                              Adrian Brock Master

                              To reproduce all the OSGi rules and old JBoss rules you would need
                              to look at the classloading rules and create AOP Domains that delegate to each other.

                              Think of A1, A2 and Support1/2 as applications. They are the
                              same application(s) deployed multiple times
                              (possibly different versions of the classes or just so the user can get their own singletons).

                              The AOPDeployer could use the classloading config and create domains
                              to match. i.e. A1 can see AOP configuration from A1 and Support1

                              For the old JBoss rules (importAll=true) then the visibilty would depend on the
                              domain name and parent policy like the old ScopedDomain.

                              The real work is going on in the "incomplete"
                              org.jboss.deployers.plugins.classloading.Module.

                              My preferred solution to use the scopes. i.e. you can choose
                              to deploy aop config at the SERVER, SUBSYSTEM, APPLICATION level, etc.
                              but this requires doing a lot work fixing everything to make sure
                              it sets the correct thread local MetaData for AOP to know which Scope
                              applies at runtime.

                              i.e. somebody could apply default aop config for some interception
                              at the server/global level and an application could override it.
                              Which AOP config applies when creating the object depends on the context of the
                              invocation NOT what classloader loads the aspect
                              (which could be in a different/global deployment) or the intercepted class (same reason)
                              to the application doing the work.

                              • 12. Re: AOP asintegration WITHOUT the integration :-)
                                Adrian Brock Master

                                 

                                "adrian@jboss.org" wrote:

                                My preferred solution to use the scopes. i.e. you can choose
                                to deploy aop config at the SERVER, SUBSYSTEM, APPLICATION level, etc.
                                but this requires doing a lot work fixing everything to make sure
                                it sets the correct thread local MetaData for AOP to know which Scope
                                applies at runtime.


                                This thead local is really just a generalization of ejbs/mbeans
                                setting the correct context classloader and java:/comp/env

                                In principle once this is done. The only work required by AOP
                                would be for the deployer to create hierarchical (same hierarchy as the scopes)
                                AOP Domains in the DeploymentUnit.get{Mutable}MetaData()
                                and deploy config into the correct scoped domain

                                The caveat being that it should be possible to override the scope,
                                e.g. a deployment could choose to deploy the aop confg into the global
                                SERVER scope or the EJB3 SUBSYSTEM scope.

                                At runtime all AOP has to do is:
                                MetaData metadata = MetaDataStack.peek();
                                AspectManager manager = metadata.getMetaData(AspectManager.class);
                                

                                which will retrieve the scoped AOP domain if there is aop config at
                                the APPLICATION/SUBSYSTEM/etc.
                                or the default AspectManager at the SERVER level if there isn't.

                                • 13. Re: AOP asintegration WITHOUT the integration :-)
                                  Kabir Khan Master

                                  How do I turn on these new classloaders in jboss-head? I'd like to see how this all hangs together...

                                  • 14. Re: AOP asintegration WITHOUT the integration :-)
                                    Kabir Khan Master

                                    Do you have some pointers to the classloading rules? I still need to get my head around this part, but in the meantime do you think we need special ClassPools as well?

                                    1 2 3 Previous Next