210470 merged into refactoring branch

9 files changed, 150 insertions, 54 deletions

diff --git a/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java b/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java
index a57bbf0ac..c7590f5cd 100644
--- a/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java
+++ b/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java
@@ -22,7 +22,6 @@ import java.util.Map;
 import java.util.Set;
 
 import org.aspectj.asm.AsmManager;
-import org.aspectj.weaver.patterns.CflowPointcut;
 import org.aspectj.weaver.patterns.DeclareParents;
 import org.aspectj.weaver.patterns.IVerificationRequired;
 import org.aspectj.weaver.tools.Trace;
diff --git a/weaver/src/org/aspectj/weaver/WeakClassLoaderReference.java b/weaver/src/org/aspectj/weaver/WeakClassLoaderReference.java
new file mode 100644
index 000000000..26812dc8e
--- /dev/null
+++ b/weaver/src/org/aspectj/weaver/WeakClassLoaderReference.java
@@ -0,0 +1,71 @@
+/* *******************************************************************
+ * Copyright (c) 2008 Contributors
+ * All rights reserved. 
+ * This program and the accompanying materials are made available 
+ * under the terms of the Eclipse Public License v1.0 
+ * which accompanies this distribution and is available at 
+ * http://www.eclipse.org/legal/epl-v10.html 
+ *  
+ * Contributors: 
+ *     Andy Clement     initial implementation 
+ * ******************************************************************/
+package org.aspectj.weaver;
+
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.WeakReference;
+
+import org.aspectj.apache.bcel.util.ClassLoaderReference;
+
+/**
+ * Wraps a reference to a classloader inside a WeakReference. This should be used where we do not want the existence of
+ * a classloader reference to prevent garbage collection of that classloader (and possibly an associated weaver instance
+ * in the case of load time weaving).
+ * <p>
+ * In more detail:<br>
+ * When load time weaving, the class Aj maintains a WeakHashMap from the classloader instance to a weaver instance. The
+ * aim is that the weaver is around as long as the classloader is and should the classloader be dereferenced then the
+ * weaver can also be garbage collected. The problem is that if there are many references to the classloader from within
+ * the weaver, these are considered hard references and cause the classloader to be long lived - even if the user of the
+ * classloader has dereferenced it in their code. The solution is that the weaver should use instances of
+ * WeakClassLoaderReference objects - so that when the users hard reference to the classloader goes, nothing in the
+ * weaver will cause it to hang around. There is a big assertion here that the WeakClassLoaderReference instances will
+ * not 'lose' their ClassLoader references until the top level ClassLoader reference is null'd. This means there is no
+ * need to check for the null case on get() in this WeakReference logic below, because we shouldn't be using this weaver
+ * if its associated ClassLoader has been collected. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=210470
+ * 
+ * 
+ * @author Andy Clement
+ */
+public class WeakClassLoaderReference implements ClassLoaderReference {
+
+    private int hashcode;
+    
+    private WeakReference loaderRef;
+    
+    public WeakClassLoaderReference(ClassLoader loader) {
+        loaderRef = new WeakReference(loader);
+        hashcode = loader.hashCode() * 37;
+    }
+
+    public WeakClassLoaderReference(ClassLoader loader, ReferenceQueue q) {
+        loaderRef = new WeakReference(loader, q);
+        hashcode = loader.hashCode() * 37;
+    }
+    
+    public ClassLoader getClassLoader() {
+        ClassLoader instance = (ClassLoader) loaderRef.get();
+        // Assert instance!=null
+        return instance;
+    }
+
+    public boolean equals(Object obj) {
+        if (!(obj instanceof WeakClassLoaderReference)) return false;
+        WeakClassLoaderReference other = (WeakClassLoaderReference) obj;
+        return (other.hashcode == hashcode);
+    }
+
+    public int hashCode() {
+        return hashcode;
+    }
+    
+}
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java b/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java
index b7b8a2115..c9b11e823 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java
@@ -39,6 +39,7 @@ import org.aspectj.apache.bcel.generic.InvokeInstruction;
 import org.aspectj.apache.bcel.generic.MULTIANEWARRAY;
 import org.aspectj.apache.bcel.generic.ObjectType;
 import org.aspectj.apache.bcel.generic.Type;
+import org.aspectj.apache.bcel.util.ClassLoaderReference;
 import org.aspectj.apache.bcel.util.ClassLoaderRepository;
 import org.aspectj.apache.bcel.util.ClassPath;
 import org.aspectj.apache.bcel.util.NonCachingClassLoaderRepository;
@@ -62,6 +63,7 @@ import org.aspectj.weaver.ResolvedMemberImpl;
 import org.aspectj.weaver.ResolvedType;
 import org.aspectj.weaver.ResolvedTypeMunger;
 import org.aspectj.weaver.UnresolvedType;
+import org.aspectj.weaver.WeakClassLoaderReference;
 import org.aspectj.weaver.World;
 import org.aspectj.weaver.AjAttribute.Aspect;
 import org.aspectj.weaver.patterns.DeclareAnnotation;
@@ -77,7 +79,7 @@ public class BcelWorld extends World implements Repository {
 	private ClassPathManager classPath;
 
     protected Repository delegate;
-    private ClassLoader loader;
+    private WeakClassLoaderReference loaderRef;
     
     
 	//private ClassPathManager aspectPath = null;
@@ -143,7 +145,7 @@ public class BcelWorld extends World implements Repository {
      */
     public BcelWorld(ClassLoader loader, IMessageHandler handler, ICrossReferenceHandler xrefHandler) {
         this.classPath = null;
-        this.loader = loader;
+        this.loaderRef = new WeakClassLoaderReference(loader);
         setMessageHandler(handler);
         setCrossReferenceHandler(xrefHandler);
         // Tell BCEL to use us for resolving any classes
@@ -152,11 +154,15 @@ public class BcelWorld extends World implements Repository {
     
     public void ensureRepositorySetup() {
     	if (delegate==null) {
-    		delegate = getClassLoaderRepositoryFor(loader);
+    		delegate = getClassLoaderRepositoryFor(loaderRef);
     	}
     }
     
-	public Repository getClassLoaderRepositoryFor(ClassLoader loader) {
+    private ClassLoader getClassLoader() {
+        return loaderRef.getClassLoader();
+    } 
+    
+    public Repository getClassLoaderRepositoryFor(ClassLoaderReference loader) {
     	if (bcelRepositoryCaching) {
     		return new ClassLoaderRepository(loader);
     	} else {
diff --git a/weaver/src/org/aspectj/weaver/bcel/Utility.java b/weaver/src/org/aspectj/weaver/bcel/Utility.java
index 766f70134..0e0323adc 100644
--- a/weaver/src/org/aspectj/weaver/bcel/Utility.java
+++ b/weaver/src/org/aspectj/weaver/bcel/Utility.java
@@ -278,6 +278,7 @@ public class Utility {
     public static String[] makeArgNames(int n) {
         String[] ret = new String[n];
         for (int i=0; i<n; i++) {
+        	// OPTIMIZE fix this occurrence and others to use constants (share)
             ret[i] = "arg" + i;
         }
         return ret;
diff --git a/weaver/src/org/aspectj/weaver/loadtime/IWeavingContext.java b/weaver/src/org/aspectj/weaver/loadtime/IWeavingContext.java
index 2ff58dcd0..8f7f72c4c 100644
--- a/weaver/src/org/aspectj/weaver/loadtime/IWeavingContext.java
+++ b/weaver/src/org/aspectj/weaver/loadtime/IWeavingContext.java
@@ -53,6 +53,8 @@ public interface IWeavingContext {
 	 */
 	public String getClassLoaderName ();
 
+    public ClassLoader getClassLoader();
+
 	/**
 	 * Format a URL
 	 * @return filename
diff --git a/weaver/src/org/aspectj/weaver/ltw/LTWWorld.java b/weaver/src/org/aspectj/weaver/ltw/LTWWorld.java
index 06fe26848..0f366cc9d 100644
--- a/weaver/src/org/aspectj/weaver/ltw/LTWWorld.java
+++ b/weaver/src/org/aspectj/weaver/ltw/LTWWorld.java
@@ -11,8 +11,6 @@
  * ******************************************************************/
 package org.aspectj.weaver.ltw;
 
-import java.lang.ref.Reference;
-import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -51,26 +49,39 @@ import org.aspectj.weaver.reflect.ReflectionWorld;
 public class LTWWorld extends BcelWorld implements IReflectionWorld {
 	
 	private AnnotationFinder annotationFinder;
-    private ClassLoader loader; // weavingContext?
     private IWeavingContext weavingContext;
+    private String classLoaderString;
+
+    private String classLoaderParentString;
+    
+    protected final static Class concurrentMapClass; 
+
+    private static final boolean ShareBootstrapTypes = false;
+    protected static Map/* <String, WeakReference<ReflectionBasedReferenceTypeDelegate>> */bootstrapTypes;
     
-    protected final static Class concurrentMapClass = makeConcurrentMapClass();
-    protected static Map/*<String, WeakReference<ReflectionBasedReferenceTypeDelegate>>*/ bootstrapTypes = makeConcurrentMap();
+    static {
+        if (ShareBootstrapTypes) {
+            concurrentMapClass = makeConcurrentMapClass();
+            bootstrapTypes = makeConcurrentMap();
+        } else {
+            concurrentMapClass = null;
+        }
+    }
     
     /**
      * Build a World from a ClassLoader, for LTW support
      */
     public LTWWorld(ClassLoader loader, IWeavingContext weavingContext, IMessageHandler handler, ICrossReferenceHandler xrefHandler) {
         super(loader, handler, xrefHandler);
-        this.loader = loader;
         this.weavingContext = weavingContext;
-        
+        classLoaderString = loader.toString();
+        classLoaderParentString = (loader.getParent() == null ? "<NullParent>" : loader.getParent().toString());
         setBehaveInJava5Way(LangUtil.is15VMOrGreater());
         annotationFinder = ReflectionWorld.makeAnnotationFinderIfAny(loader, this);
     }
     
 	public ClassLoader getClassLoader() {
-        return this.loader;
+        return weavingContext.getClassLoader();
     }
     
     //TEST
@@ -99,30 +110,33 @@ public class LTWWorld extends BcelWorld implements IReflectionWorld {
 
     protected ReferenceTypeDelegate resolveIfBootstrapDelegate(ReferenceType ty) {
         // first check for anything available in the bootstrap loader: these types are just defined from that without allowing nondelegation
-    	String name = ty.getName();
-        Reference bootRef = (Reference)bootstrapTypes.get(name);
-        if (bootRef != null) {        	
-        	ReferenceTypeDelegate rtd = (ReferenceTypeDelegate)bootRef.get();
-        	if (rtd != null) {
-        		return rtd;
-        	}
-        }
-
-        char fc = name.charAt(0);
-        if (fc=='j' || fc=='c' || fc=='o' || fc=='s') { // cheaper than imminent string startsWith tests
-	        if (name.startsWith("java") || name.startsWith("com.sun.") || name.startsWith("org.w3c") || 
-	        	name.startsWith("sun.") || name.startsWith("org.omg")) {
-		        ReferenceTypeDelegate bootstrapLoaderDelegate = resolveReflectionTypeDelegate(ty, null);
-		        if (bootstrapLoaderDelegate != null) {
-		            // it's always fine to load these bytes: there's no weaving into them
-		            // and since the class isn't initialized, all we are doing at this point is loading the bytes
-		            //processedRefTypes.put(ty, this); // has no effect - and probably too aggressive if we did store these in the type map            
-		        	// should we share these, like we do the BCEL delegates?
-		        	bootstrapTypes.put(ty.getName(), new WeakReference(bootstrapLoaderDelegate));
-		        }    	
-		        return bootstrapLoaderDelegate;
-	        }
-        }
+// if (!ShareBootstrapTypes) return null;
+// String name = ty.getName();
+        // Reference bootRef = (Reference) bootstrapTypes.get(name);
+        // if (bootRef != null) {
+        // ReferenceTypeDelegate rtd = (ReferenceTypeDelegate) bootRef.get();
+        // if (rtd != null) {
+        // return rtd;
+        // }
+        // }
+        //
+        // char fc = name.charAt(0);
+        // if (fc == 'j' || fc == 'c' || fc == 'o' || fc == 's') { // cheaper than imminent string startsWith tests
+        // if (name.startsWith("java") || name.startsWith("com.sun.") || name.startsWith("org.w3c") ||
+        // name.startsWith("sun.") || name.startsWith("org.omg")) {
+        // ReferenceTypeDelegate bootstrapLoaderDelegate = resolveReflectionTypeDelegate(ty, null);
+        // if (bootstrapLoaderDelegate != null) {
+        // // it's always fine to load these bytes: there's no weaving into them
+        // // and since the class isn't initialized, all we are doing at this point is loading the bytes
+        // // processedRefTypes.put(ty, this); // has no effect - and probably too aggressive if we did store
+        // // these in the type map
+        //
+        // // should we share these, like we do the BCEL delegates?
+        // bootstrapTypes.put(ty.getName(), new WeakReference(bootstrapLoaderDelegate));
+        // }
+        // return bootstrapLoaderDelegate;
+        // }
+        // }
         return null;
     }
     
@@ -241,7 +255,7 @@ public class LTWWorld extends BcelWorld implements IReflectionWorld {
 		if (ret.isParameterizedType() || ret.isGenericType()) {
 			toResolve = toResolve.getGenericType();
 		}
-		ReferenceTypeDelegate rtd = resolveReflectionTypeDelegate((ReferenceType)toResolve,loader);
+		ReferenceTypeDelegate rtd = resolveReflectionTypeDelegate((ReferenceType)toResolve,getClassLoader());
 		((ReferenceType)ret).setDelegate(rtd);
 		return ret;
 	}
@@ -253,9 +267,9 @@ public class LTWWorld extends BcelWorld implements IReflectionWorld {
 
 	public void accept(IVisitor visitor) {
 		visitor.visitObject("Class loader:");
-		visitor.visitObject(loader);
+		visitor.visitObject(classLoaderString);
 		visitor.visitObject("Class loader parent:");
-		visitor.visitObject(loader.getParent());
+		visitor.visitObject(classLoaderParentString);
 		super.accept(visitor);
 	}
 	 public boolean shouldGoForIt() {
diff --git a/weaver/src/org/aspectj/weaver/reflect/ReflectionBasedReferenceTypeDelegate.java b/weaver/src/org/aspectj/weaver/reflect/ReflectionBasedReferenceTypeDelegate.java
index 01c16ef82..1e3628c47 100644
--- a/weaver/src/org/aspectj/weaver/reflect/ReflectionBasedReferenceTypeDelegate.java
+++ b/weaver/src/org/aspectj/weaver/reflect/ReflectionBasedReferenceTypeDelegate.java
@@ -30,6 +30,7 @@ import org.aspectj.weaver.ResolvedType;
 import org.aspectj.weaver.SourceContextImpl;
 import org.aspectj.weaver.TypeVariable;
 import org.aspectj.weaver.UnresolvedType;
+import org.aspectj.weaver.WeakClassLoaderReference;
 import org.aspectj.weaver.WeaverStateInfo;
 import org.aspectj.weaver.World;
 import org.aspectj.weaver.patterns.PerClause;
@@ -42,10 +43,10 @@ import org.aspectj.weaver.patterns.PerClause;
  */
 public class ReflectionBasedReferenceTypeDelegate implements ReferenceTypeDelegate {
 
-	private static final ClassLoader BootClassLoader = new URLClassLoader(new URL[0]);
+	private static final ClassLoader BootClassLoader = new URLClassLoader(new URL[0]);// ReflectionBasedReferenceTypeDelegate.class.getClassLoader();
 	
 	protected Class myClass = null;
-	protected ClassLoader classLoader = null;
+	protected WeakClassLoaderReference classLoaderReference = null;
 	private World world;
 	private ReferenceType resolvedType;
 	private ResolvedMember[] fields = null;
@@ -63,7 +64,7 @@ public class ReflectionBasedReferenceTypeDelegate implements ReferenceTypeDelega
 		this.myClass = aClass;
 		this.resolvedType = aType;
 		this.world = aWorld;
-		this.classLoader = (aClassLoader != null) ? aClassLoader : BootClassLoader;
+		this.classLoaderReference = new WeakClassLoaderReference((aClassLoader != null) ? aClassLoader : BootClassLoader);
 	}
 	
 	protected Class getBaseClass() { 
diff --git a/weaver/src/org/aspectj/weaver/reflect/ReflectionWorld.java b/weaver/src/org/aspectj/weaver/reflect/ReflectionWorld.java
index 7994d8f39..12107649c 100644
--- a/weaver/src/org/aspectj/weaver/reflect/ReflectionWorld.java
+++ b/weaver/src/org/aspectj/weaver/reflect/ReflectionWorld.java
@@ -25,6 +25,7 @@ import org.aspectj.weaver.ResolvedMember;
 import org.aspectj.weaver.ResolvedType;
 import org.aspectj.weaver.ResolvedTypeMunger;
 import org.aspectj.weaver.UnresolvedType;
+import org.aspectj.weaver.WeakClassLoaderReference;
 import org.aspectj.weaver.World;
 import org.aspectj.weaver.AjAttribute.AdviceAttribute;
 import org.aspectj.weaver.patterns.Pointcut;
@@ -38,23 +39,23 @@ import org.aspectj.weaver.patterns.PerClause.Kind;
  */
 public class ReflectionWorld extends World implements IReflectionWorld {
 
-	private ClassLoader classLoader;
+	private WeakClassLoaderReference classLoaderReference;
 	private AnnotationFinder annotationFinder;
 	
 	private ReflectionWorld() {
 		super();
 		this.setMessageHandler(new ExceptionBasedMessageHandler());
 		setBehaveInJava5Way(LangUtil.is15VMOrGreater());
-		this.classLoader = ReflectionWorld.class.getClassLoader();
-		this.annotationFinder = makeAnnotationFinderIfAny(classLoader, this);
+		this.classLoaderReference = new WeakClassLoaderReference(ReflectionWorld.class.getClassLoader());
+		this.annotationFinder = makeAnnotationFinderIfAny(classLoaderReference.getClassLoader(), this);
 	}
 	
 	public ReflectionWorld(ClassLoader aClassLoader) {
 		super();
 		this.setMessageHandler(new ExceptionBasedMessageHandler());
 		setBehaveInJava5Way(LangUtil.is15VMOrGreater());
-		this.classLoader = aClassLoader;
-		this.annotationFinder = makeAnnotationFinderIfAny(classLoader, this);
+		this.classLoaderReference = new WeakClassLoaderReference(aClassLoader);
+		this.annotationFinder = makeAnnotationFinderIfAny(classLoaderReference.getClassLoader(), this);
 	}
 
 	public static AnnotationFinder makeAnnotationFinderIfAny(ClassLoader loader, World world) {
@@ -78,7 +79,7 @@ public class ReflectionWorld extends World implements IReflectionWorld {
 	}
 	
 	public ClassLoader getClassLoader() {
-		return this.classLoader;
+		return this.classLoaderReference.getClassLoader();
 	}
 	
 	public AnnotationFinder getAnnotationFinder() {
@@ -107,7 +108,7 @@ public class ReflectionWorld extends World implements IReflectionWorld {
 	 * @see org.aspectj.weaver.World#resolveDelegate(org.aspectj.weaver.ReferenceType)
 	 */
 	protected ReferenceTypeDelegate resolveDelegate(ReferenceType ty) {
-		return ReflectionBasedReferenceTypeDelegateFactory.createDelegate(ty, this, this.classLoader);
+		return ReflectionBasedReferenceTypeDelegateFactory.createDelegate(ty, this, this.classLoaderReference.getClassLoader());
 	}
 
 	/* (non-Javadoc)
diff --git a/weaver/src/org/aspectj/weaver/tools/PointcutParser.java b/weaver/src/org/aspectj/weaver/tools/PointcutParser.java
index 3d22f7aae..f6ff5ba03 100644
--- a/weaver/src/org/aspectj/weaver/tools/PointcutParser.java
+++ b/weaver/src/org/aspectj/weaver/tools/PointcutParser.java
@@ -28,6 +28,7 @@ import org.aspectj.weaver.IntMap;
 import org.aspectj.weaver.ResolvedType;
 import org.aspectj.weaver.Shadow;
 import org.aspectj.weaver.UnresolvedType;
+import org.aspectj.weaver.WeakClassLoaderReference;
 import org.aspectj.weaver.World;
 import org.aspectj.weaver.bcel.AtAjAttributes;
 import org.aspectj.weaver.internal.tools.PointcutExpressionImpl;
@@ -56,7 +57,7 @@ import org.aspectj.weaver.reflect.ReflectionWorld;
 public class PointcutParser {
     
 	private ReflectionWorld world;
-	private ClassLoader classLoader;
+	private WeakClassLoaderReference classLoaderReference;
     private Set supportedPrimitives; 
     private Set pointcutDesignators = new HashSet();
     
@@ -224,8 +225,8 @@ public class PointcutParser {
      * @param aLoader
      */
     protected void setClassLoader(ClassLoader aLoader) {
-    	this.classLoader = aLoader;
-    	world = new ReflectionWorld(this.classLoader);
+    	this.classLoaderReference = new WeakClassLoaderReference(aLoader);
+    	world = new ReflectionWorld(this.classLoaderReference.getClassLoader());
     }
 
     /**
@@ -235,7 +236,7 @@ public class PointcutParser {
      * lint properties
      */
     public void setLintProperties(String resourcePath)throws IOException {
-    	URL url = this.classLoader.getResource(resourcePath);
+    	URL url = this.classLoaderReference.getClassLoader().getResource(resourcePath);
     	InputStream is = url.openStream();
     	Properties p = new Properties();
 		p.load(is);