[jboss-cvs] JBossAS SVN: r63899 - in projects/aop/trunk: aop/src/main/org/jboss/aop/classpool and 1 other directories.

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Sat Jul 7 06:02:57 EDT 2007


Author: adrian at jboss.org
Date: 2007-07-07 06:02:57 -0400 (Sat, 07 Jul 2007)
New Revision: 63899

Added:
   projects/aop/trunk/aop/src/main/org/jboss/aop/classpool/AOPClassLoaderScopingPolicy.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/classpool/AOPScopedClassLoaderHelperBridge.java
Modified:
   projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java
   projects/aop/trunk/aop/src/main/org/jboss/aop/Domain.java
   projects/aop/trunk/asintegration/src/main/org/jboss/aop/deployment/AspectManagerService.java
   projects/aop/trunk/asintegration/src/main/org/jboss/aop/deployment/JBossClassPoolFactory.java
Log:
[JBAOP-107] - Rework the scope policy to move more of the policy into the abstraction.
AOPScopedClassLoaderHelper is now an implementation detail of AOPClassLoaderScopingPolicy

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java	2007-07-07 03:36:03 UTC (rev 63898)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/AspectManager.java	2007-07-07 10:02:57 UTC (rev 63899)
@@ -32,14 +32,19 @@
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.WeakHashMap;
-import java.util.List;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
+import javassist.ClassPool;
+import javassist.CtClass;
+import javassist.scopedpool.ScopedClassPool;
+import javassist.scopedpool.ScopedClassPoolFactory;
+
 import org.jboss.aop.advice.AdviceBinding;
 import org.jboss.aop.advice.AdviceStack;
 import org.jboss.aop.advice.AspectDefinition;
@@ -50,6 +55,7 @@
 import org.jboss.aop.advice.PrecedenceDefEntry;
 import org.jboss.aop.advice.PrecedenceSorter;
 import org.jboss.aop.advice.Scope;
+import org.jboss.aop.classpool.AOPClassLoaderScopingPolicy;
 import org.jboss.aop.classpool.AOPClassPoolRepository;
 import org.jboss.aop.classpool.AOPScopedClassLoaderHelper;
 import org.jboss.aop.instrument.GeneratedAdvisorInstrumentor;
@@ -78,10 +84,6 @@
 import org.jboss.util.collection.WeakValueHashMap;
 import org.jboss.util.loading.Translatable;
 import org.jboss.util.loading.Translator;
-import javassist.ClassPool;
-import javassist.CtClass;
-import javassist.scopedpool.ScopedClassPool;
-import javassist.scopedpool.ScopedClassPoolFactory;
 
 /**
  * This singleton class manages all pointcuts and metadata.
@@ -95,6 +97,7 @@
  * to do that.
  *
  * @author <a href="mailto:bill at jboss.org">Bill Burke</a>
+ * @author adrian at jboss.org
  * @version $Revision$
  */
 public class AspectManager
@@ -109,6 +112,9 @@
    protected final WeakHashMap advisors = new WeakHashMap();
    
    /** A map of domains by loader repository, maintaned by the top level AspectManager */
+   // This seems to be maintained but not used, to avoid garbage collection of domains?
+   // I moved it to the AOPClassLoaderScopingPolicy for now 
+   @Deprecated 
    protected WeakHashMap scopedClassLoaderDomains = UnmodifiableEmptyCollections.EMPTY_WEAK_HASHMAP;
 
    /** A map of domains by class, maintaned by the top level AspectManager */
@@ -174,9 +180,13 @@
    // indicates that the transformation process has begun
    protected boolean transformationStarted = false;
 
-   //This will be set by the AspectManagerService if running in JBoss
-   public static AOPScopedClassLoaderHelper scopedCLHelper;
+   @Deprecated // replaced by the temporary AOPClassLoaderScopingPolicy - no longer referenced
+   protected static AOPScopedClassLoaderHelper scopedCLHelper;
    
+   /** The classloader scoping policy */
+   // This shouldn't really be static (artifact of singleton and self-bootstrap design)
+   private static AOPClassLoaderScopingPolicy classLoaderScopingPolicy;
+   
    //Keeps track of if we need to convert references etc for a given class. Domains for scoped classloaders will have their own version of this
    protected static InterceptionMarkers interceptionMarkers = new InterceptionMarkers();
    
@@ -195,30 +205,63 @@
     */
    public static boolean verbose = false;
 
+   /**
+    * Get the top level aspect manager
+    * 
+    * @return the top level aspect manager
+    */
    public static synchronized AspectManager getTopLevelAspectManager()
    {
-      if (scopedCLHelper == null)
+      if (classLoaderScopingPolicy == null)
       {
          //We are not running in jboss
          return instance();
       }
-      ClassLoader topUcl = scopedCLHelper.getTopLevelJBossClassLoader();
-      return instance(topUcl);
 
+      AspectManager result = initManager();
+      Domain scopedDomain = classLoaderScopingPolicy.getTopLevelDomain(result);
+      if (scopedDomain != null)
+         result = scopedDomain;
+      return result;
    }
 
    public static synchronized AspectManager instance()
    {
       return instance(Thread.currentThread().getContextClassLoader());
    }
-
+   
+   /**
+    * Get the aspect manager for a classloader
+    * 
+    * @param loadingClassLoader the classloader
+    * @return the aspect manager
+    */
    public static synchronized AspectManager instance(ClassLoader loadingClassLoader)
    {
+      AspectManager result = initManager();
+      if (classLoaderScopingPolicy != null)
+      {
+         Domain scopedDomain = classLoaderScopingPolicy.getDomain(loadingClassLoader, result);
+         if (scopedDomain != null)
+            result = scopedDomain;
+      }
+      return result;
+   }
+
+   /**
+    * Initialise the manager if not already dones so<p>
+    * 
+    * This method should be invoked in a synchronized block
+    * 
+    * @return the manager
+    */
+   private static AspectManager initManager()
+   {
       if (manager == null)
       {
-         AccessController.doPrivileged(new PrivilegedAction()
+         AccessController.doPrivileged(new PrivilegedAction<AspectManager>()
          {
-            public Object run()
+            public AspectManager run()
             {
                String optimized = System.getProperty("jboss.aop.optimized", null);
                if (optimized != null)
@@ -232,7 +275,6 @@
                }
                manager = new AspectManager();
                //Initialise frequently used fields needed by the top-level manager
-               manager.scopedClassLoaderDomains = new WeakHashMap();
                manager.subDomainsPerClass = new WeakHashMap();
                manager.exclude = new ArrayList();
                manager.include = new ArrayList();
@@ -302,37 +344,30 @@
             }
          });
       }
-
-      if (scopedCLHelper != null)
-      {
-         ClassLoader scopedClassLoader = scopedCLHelper.ifScopedDeploymentGetScopedParentUclForCL(loadingClassLoader);
-         if (scopedClassLoader != null)
-         {
-            Domain scopedManager = null;
-            synchronized (AOPClassPoolRepository.getInstance().getRegisteredCLs())
-            {
-               Object loaderRepository = scopedCLHelper.getLoaderRepository(loadingClassLoader);
-               scopedManager = (Domain)manager.scopedClassLoaderDomains.get(loaderRepository);
-               if (scopedManager == null)
-               {
-                  scopedManager = scopedCLHelper.getScopedClassLoaderDomain(scopedClassLoader, manager);
-                  if (verbose && logger.isDebugEnabled())
-                  {
-                     logger.debug("Created domain " + scopedManager + " for scoped deployment on: " +
-                           loadingClassLoader + "; identifying scoped ucl: " + scopedClassLoader);
-                  }
-                  scopedManager.setInheritsBindings(true);
-                  scopedManager.setInheritsDeclarations(true);
-                  
-                  manager.scopedClassLoaderDomains.put(loaderRepository, scopedManager);
-               }
-            }
-            return scopedManager;
-         }
-      }
       return manager;
    }
 
+   /**
+    * Get the classLoaderScopingPolicy.
+    * 
+    * @return the classLoaderScopingPolicy.
+    */
+   public static AOPClassLoaderScopingPolicy getClassLoaderScopingPolicy()
+   {
+      return classLoaderScopingPolicy;
+   }
+
+   /**
+    * Set the classLoaderScopingPolicy.
+    * 
+    * TODO does it make sense for this to be modified once it has been set?
+    * @param classLoaderScopingPolicy the classLoaderScopingPolicy.
+    */
+   public static void setClassLoaderScopingPolicy(AOPClassLoaderScopingPolicy classLoaderScopingPolicy)
+   {
+      AspectManager.classLoaderScopingPolicy = classLoaderScopingPolicy;
+   }
+
    public InterceptionMarkers getInterceptionMarkers()
    {
       return interceptionMarkers;
@@ -696,6 +731,8 @@
       AOPClassPoolRepository.getInstance().registerClass(clazz);
    }
 
+   @Deprecated
+   // This is never invoked, it always referenced the scopedClassLoaderDomains directly?
    protected Map getScopedClassLoaderDomains()
    {
       return scopedClassLoaderDomains;

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/Domain.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/Domain.java	2007-07-07 03:36:03 UTC (rev 63898)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/Domain.java	2007-07-07 10:02:57 UTC (rev 63899)
@@ -50,6 +50,7 @@
  * Comment
  *
  * @author <a href="mailto:bill at jboss.org">Bill Burke</a>
+ * @author adrian at jboss.org
  * @version $Revision$
  */
 public class Domain extends AspectManager
@@ -934,7 +935,7 @@
    }
 
 
-   /** Managed by the top-level aspect manager */
+   @Deprecated
    protected Map getScopedClassLoaderDomains()
    {
       return parent.getScopedClassLoaderDomains();

Added: projects/aop/trunk/aop/src/main/org/jboss/aop/classpool/AOPClassLoaderScopingPolicy.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/classpool/AOPClassLoaderScopingPolicy.java	                        (rev 0)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/classpool/AOPClassLoaderScopingPolicy.java	2007-07-07 10:02:57 UTC (rev 63899)
@@ -0,0 +1,64 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2006, JBoss Inc., and individual contributors as indicated
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* This is free software; you can redistribute it and/or modify it
+* under the terms of the GNU Lesser General Public License as
+* published by the Free Software Foundation; either version 2.1 of
+* the License, or (at your option) any later version.
+*
+* This software is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+* Lesser General Public License for more details.
+*
+* You should have received a copy of the GNU Lesser General Public
+* License along with this software; if not, write to the Free
+* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+*/
+package org.jboss.aop.classpool;
+
+import org.jboss.aop.AspectManager;
+import org.jboss.aop.Domain;
+
+/**
+ * AOPClassLoaderScopingPolicy.<p>
+ * 
+ * This is a temporary abstraction to replace AOPScopedClassLoaderHelper
+ * until a better deployment/context resolution mechanism is used rather
+ * than using the classloader to decide things.
+ * 
+ * @author <a href="adrian at jboss.com">Adrian Brock</a>
+ * @version $Revision: 1.1 $
+ */
+public interface AOPClassLoaderScopingPolicy
+{
+   /**
+    * Is the classloader scoped?
+    *
+    * TODO Move ClassPool construction policy into here and remove this
+    * @param classLoader the classLoader
+    * @return true when scoped 
+    */
+   boolean isScoped(ClassLoader classLoader);
+   
+   /**
+    * Get the domain for classloader
+    * 
+    * @param classLoader the classloader
+    * @param parent the parent (isn't this always the top level aspect manager?)
+    * @return any scoped domain or the null if not scoped
+    */
+   Domain getDomain(ClassLoader classLoader, AspectManager parent);
+
+   /**
+    * Get the top level domain
+    * 
+    * @param parent the parent (isn't this always the top level aspect manager?)
+    * @return any scoped domain or the null if not scoped
+    */
+   Domain getTopLevelDomain(AspectManager parent);
+}

Added: projects/aop/trunk/aop/src/main/org/jboss/aop/classpool/AOPScopedClassLoaderHelperBridge.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/classpool/AOPScopedClassLoaderHelperBridge.java	                        (rev 0)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/classpool/AOPScopedClassLoaderHelperBridge.java	2007-07-07 10:02:57 UTC (rev 63899)
@@ -0,0 +1,100 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2006, JBoss Inc., and individual contributors as indicated
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* This is free software; you can redistribute it and/or modify it
+* under the terms of the GNU Lesser General Public License as
+* published by the Free Software Foundation; either version 2.1 of
+* the License, or (at your option) any later version.
+*
+* This software is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+* Lesser General Public License for more details.
+*
+* You should have received a copy of the GNU Lesser General Public
+* License along with this software; if not, write to the Free
+* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+*/
+package org.jboss.aop.classpool;
+
+import java.util.Map;
+import java.util.WeakHashMap;
+
+import org.jboss.aop.AspectManager;
+import org.jboss.aop.Domain;
+import org.jboss.logging.Logger;
+
+/**
+ * AOPScopedClassLoaderHelperBridge.<p>
+ * 
+ * A bridged from the old scoped classloader helper to the temporary
+ * AOPClassLoaderScopingPolicy
+ * 
+ * @author <a href="adrian at jboss.com">Adrian Brock</a>
+ * @version $Revision: 1.1 $
+ */
+public class AOPScopedClassLoaderHelperBridge implements AOPClassLoaderScopingPolicy
+{
+   /** The log */
+   private static final Logger log = Logger.getLogger(AOPScopedClassLoaderHelper.class);
+
+   /** The delegate */
+   private AOPScopedClassLoaderHelper delegate;
+   
+   /** A map of domains by loader repository, maintaned by the top level AspectManager */
+   private Map<Object, Domain> scopedClassLoaderDomains = new WeakHashMap<Object, Domain>();
+
+   /**
+    * Create a new AOPScopedClassLoaderHelperBridge.
+    * 
+    * @param delegate the delegate helper
+    * @throws IllegalArgumentException for a null delegate
+    */
+   public AOPScopedClassLoaderHelperBridge(AOPScopedClassLoaderHelper delegate)
+   {
+      if (delegate == null)
+         throw new IllegalArgumentException("Null delegate");
+      this.delegate = delegate;
+   }
+   
+   public boolean isScoped(ClassLoader classLoader)
+   {
+      return delegate.ifScopedDeploymentGetScopedParentUclForCL(classLoader) != null;
+   }
+
+   public synchronized Domain getDomain(ClassLoader classLoader, AspectManager parent)
+   {
+      ClassLoader scopedClassLoader = delegate.ifScopedDeploymentGetScopedParentUclForCL(classLoader);
+      if (scopedClassLoader != null)
+      {
+         Domain scopedManager = null;
+         synchronized (AOPClassPoolRepository.getInstance().getRegisteredCLs())
+         {
+            Object loaderRepository = delegate.getLoaderRepository(classLoader);
+            scopedManager = scopedClassLoaderDomains.get(loaderRepository);
+            if (scopedManager == null)
+            {
+               scopedManager = delegate.getScopedClassLoaderDomain(scopedClassLoader, parent);
+               log.debug("Created domain " + scopedManager + " for scoped deployment on: " +
+                        classLoader + "; identifying scoped ucl: " + scopedClassLoader);
+               scopedManager.setInheritsBindings(true);
+               scopedManager.setInheritsDeclarations(true);
+               
+               scopedClassLoaderDomains.put(loaderRepository, scopedManager);
+            }
+            return scopedManager;
+         }
+      }
+      return null;
+   }
+
+   public Domain getTopLevelDomain(AspectManager parent)
+   {
+      ClassLoader classLoader = delegate.getTopLevelJBossClassLoader();
+      return getDomain(classLoader, parent);
+   }
+}

Modified: projects/aop/trunk/asintegration/src/main/org/jboss/aop/deployment/AspectManagerService.java
===================================================================
--- projects/aop/trunk/asintegration/src/main/org/jboss/aop/deployment/AspectManagerService.java	2007-07-07 03:36:03 UTC (rev 63898)
+++ projects/aop/trunk/asintegration/src/main/org/jboss/aop/deployment/AspectManagerService.java	2007-07-07 10:02:57 UTC (rev 63899)
@@ -39,6 +39,7 @@
 import javax.management.Notification;
 import javax.management.ObjectName;
 import javax.management.ReflectionException;
+
 import org.jboss.aop.AspectManager;
 import org.jboss.aop.AspectNotificationHandler;
 import org.jboss.aop.AspectXmlLoader;
@@ -46,6 +47,9 @@
 import org.jboss.aop.ClassicWeavingStrategy;
 import org.jboss.aop.Deployment;
 import org.jboss.aop.SuperClassesFirstWeavingStrategy;
+import org.jboss.aop.classpool.AOPClassLoaderScopingPolicy;
+import org.jboss.aop.classpool.AOPScopedClassLoaderHelper;
+import org.jboss.aop.classpool.AOPScopedClassLoaderHelperBridge;
 import org.jboss.aop.hook.JDK14Transformer;
 import org.jboss.aop.hook.JDK14TransformerManager;
 import org.jboss.aop.instrument.InstrumentorFactory;
@@ -58,6 +62,7 @@
 
 /**
  * @author <a href="mailto:bill at jboss.org">Bill Burke</a>
+ * @author adrian at jboss.org
  * @version $Revision$
  * @jmx:mbean extends="org.jboss.system.ServiceMBean"
  */
@@ -159,7 +164,9 @@
       created = true;
       AspectManager.notificationHandler = this;
 
-      AspectManager.scopedCLHelper = new JBossScopedClassLoaderHelper();
+      AOPScopedClassLoaderHelper helper = new JBossScopedClassLoaderHelper();
+      AOPClassLoaderScopingPolicy policy = new AOPScopedClassLoaderHelperBridge(helper);
+      AspectManager.setClassLoaderScopingPolicy(policy);
 
       baseAop();
    }

Modified: projects/aop/trunk/asintegration/src/main/org/jboss/aop/deployment/JBossClassPoolFactory.java
===================================================================
--- projects/aop/trunk/asintegration/src/main/org/jboss/aop/deployment/JBossClassPoolFactory.java	2007-07-07 03:36:03 UTC (rev 63898)
+++ projects/aop/trunk/asintegration/src/main/org/jboss/aop/deployment/JBossClassPoolFactory.java	2007-07-07 10:02:57 UTC (rev 63899)
@@ -26,6 +26,7 @@
 import java.net.URL;
 
 import org.jboss.aop.AspectManager;
+import org.jboss.aop.classpool.AOPClassLoaderScopingPolicy;
 import org.jboss.aop.classpool.AOPClassPool;
 import org.jboss.mx.loading.RepositoryClassLoader;
 import javassist.ClassPool;
@@ -37,6 +38,7 @@
  * Comment
  *
  * @author <a href="mailto:bill at jboss.org">Bill Burke</a>
+ * @author adrian at jboss.org
  * @version $Revision$
  *
  **/
@@ -63,7 +65,8 @@
          {
             throw new RuntimeException(e);
          }
-         if (AspectManager.scopedCLHelper.ifScopedDeploymentGetScopedParentUclForCL(cl) != null)
+         AOPClassLoaderScopingPolicy policy = AspectManager.getClassLoaderScopingPolicy();
+         if (policy != null && policy.isScoped(cl))
          {
             //It is scoped
             return new ScopedJBossClassPool(cl, src, repository, tempdir, tmpCP);




More information about the jboss-cvs-commits mailing list