test and fix for 160674: simpler strategy for loading bytecode on SAPBEFORE_133532

12 files changed, 249 insertions, 7 deletions

diff --git a/bcel-builder/src/org/aspectj/apache/bcel/util/ClassLoaderRepository.java b/bcel-builder/src/org/aspectj/apache/bcel/util/ClassLoaderRepository.java
index 2f7325186..f7cdee26b 100644
--- a/bcel-builder/src/org/aspectj/apache/bcel/util/ClassLoaderRepository.java
+++ b/bcel-builder/src/org/aspectj/apache/bcel/util/ClassLoaderRepository.java
@@ -80,7 +80,7 @@ import org.aspectj.apache.bcel.classfile.JavaClass;
  *
  * @see org.aspectj.apache.bcel.Repository
  *
- * @version $Id: ClassLoaderRepository.java,v 1.8 2006/08/21 15:23:58 aclement Exp $
+ * @version $Id: ClassLoaderRepository.java,v 1.9 2006/10/12 19:58:18 aclement Exp $
  * @author <A HREF="mailto:markus.dahm@berlin.de">M. Dahm</A>
  * @author David Dixon-Peugh
  */
@@ -261,7 +261,7 @@ public class ClassLoaderRepository implements Repository {
 	long time = System.currentTimeMillis();
     java.net.URL url = toURL(className);
 	timeManipulatingURLs += (System.currentTimeMillis() - time);
-	if (url==null) throw new ClassNotFoundException(className + " not found.");
+	if (url==null) throw new ClassNotFoundException(className + " not found - unable to determine URL");
     
 	JavaClass clazz = null;
 
@@ -282,7 +282,7 @@ public class ClassLoaderRepository implements Repository {
 	    String classFile = className.replace('.', '/');
 		InputStream is = (useSharedCache?url.openStream():loader.getResourceAsStream( classFile + ".class" ));
 	    if (is == null) { 
-		  throw new ClassNotFoundException(className + " not found.");
+		  throw new ClassNotFoundException(className + " not found "+(url==null?"":"- using url "+url));
 	    }
 		ClassParser parser = new ClassParser( is, className );
         clazz = parser.parse();
diff --git a/bcel-builder/src/org/aspectj/apache/bcel/util/NonCachingClassLoaderRepository.java b/bcel-builder/src/org/aspectj/apache/bcel/util/NonCachingClassLoaderRepository.java
new file mode 100644
index 000000000..20c82006d
--- /dev/null
+++ b/bcel-builder/src/org/aspectj/apache/bcel/util/NonCachingClassLoaderRepository.java
@@ -0,0 +1,167 @@
+package org.aspectj.apache.bcel.util;
+
+/* ====================================================================
+ * The Apache Software License, Version 1.1
+ *
+ * Copyright (c) 2001 The Apache Software Foundation.  All rights
+ * reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * 3. The end-user documentation included with the redistribution,
+ *    if any, must include the following acknowledgment:
+ *       "This product includes software developed by the
+ *        Apache Software Foundation (http://www.apache.org/)."
+ *    Alternately, this acknowledgment may appear in the software itself,
+ *    if and wherever such third-party acknowledgments normally appear.
+ *
+ * 4. The names "Apache" and "Apache Software Foundation" and
+ *    "Apache BCEL" must not be used to endorse or promote products
+ *    derived from this software without prior written permission. For
+ *    written permission, please contact apache@apache.org.
+ *
+ * 5. Products derived from this software may not be called "Apache",
+ *    "Apache BCEL", nor may "Apache" appear in their name, without
+ *    prior written permission of the Apache Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED.  IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
+ * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ */
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.WeakHashMap;
+
+import org.aspectj.apache.bcel.classfile.ClassParser;
+import org.aspectj.apache.bcel.classfile.JavaClass;
+
+/**
+ * The repository maintains information about which classes have
+ * been loaded.
+ *
+ * It loads its data from the ClassLoader implementation
+ * passed into its constructor.
+ *
+ * @see org.aspectj.apache.bcel.Repository
+ *
+ * @version $Id: NonCachingClassLoaderRepository.java,v 1.1 2006/10/12 19:58:18 aclement Exp $
+ * @author <A HREF="mailto:markus.dahm@berlin.de">M. Dahm</A>
+ * @author David Dixon-Peugh
+ * 
+ */
+public class NonCachingClassLoaderRepository
+  implements Repository
+{  
+  private static java.lang.ClassLoader bootClassLoader = null;
+
+  private java.lang.ClassLoader loader;
+  private WeakHashMap loadedClasses =
+    new WeakHashMap(); // CLASSNAME X JAVACLASS
+
+  public NonCachingClassLoaderRepository( java.lang.ClassLoader loader ) {
+	    this.loader = (loader != null) ? loader : getBootClassLoader();
+  }
+
+  private static synchronized java.lang.ClassLoader getBootClassLoader() {
+	  if (bootClassLoader == null) {
+		  bootClassLoader = new URLClassLoader(new URL[0]);
+	  }
+	  return bootClassLoader;
+  }
+  
+  /**
+   * Store a new JavaClass into this Repository.
+   */
+  public void storeClass( JavaClass clazz ) {
+    loadedClasses.put( clazz.getClassName(),
+		       clazz );
+    clazz.setRepository( this );
+  }
+
+  /**
+   * Remove class from repository
+   */
+  public void removeClass(JavaClass clazz) {
+    loadedClasses.remove(clazz.getClassName());
+  }
+
+  /**
+   * Find an already defined JavaClass.
+   */
+  public JavaClass findClass( String className ) {
+    if ( loadedClasses.containsKey( className )) {
+      return (JavaClass) loadedClasses.get( className );
+    } else {
+      return null;
+    }
+  }
+
+  /**
+   * Lookup a JavaClass object from the Class Name provided.
+   */
+  public JavaClass loadClass( String className ) 
+    throws ClassNotFoundException
+  {
+    String classFile = className.replace('.', '/');
+
+    JavaClass RC = findClass( className );
+    if (RC != null) { return RC; }
+
+    try {
+      InputStream is = 
+	loader.getResourceAsStream( classFile + ".class" );
+	    
+      if(is == null) {
+	throw new ClassNotFoundException(className + " not found.");
+      }
+
+      ClassParser parser = new ClassParser( is, className );
+      RC = parser.parse();
+	    
+      storeClass( RC );
+
+      return RC;
+    } catch (IOException e) {
+      throw new ClassNotFoundException( e.toString() );
+    }
+  }
+
+  public JavaClass loadClass(Class clazz) throws ClassNotFoundException {
+    return loadClass(clazz.getName());
+  }
+
+  /** Clear all entries from cache.
+   */
+  public void clear() {
+    loadedClasses.clear();
+  }
+}
+
diff --git a/lib/bcel/bcel-src.zip b/lib/bcel/bcel-src.zip
index 0874f3767..54bfb3bcb 100644
--- a/lib/bcel/bcel-src.zip
+++ b/lib/bcel/bcel-src.zip
diff --git a/lib/bcel/bcel.jar b/lib/bcel/bcel.jar
index b07a9820d..d49a8846c 100644
--- a/lib/bcel/bcel.jar
+++ b/lib/bcel/bcel.jar
diff --git a/tests/bugs153/pr160674/MyAspect.java b/tests/bugs153/pr160674/MyAspect.java
new file mode 100644
index 000000000..cbad3a906
--- /dev/null
+++ b/tests/bugs153/pr160674/MyAspect.java
@@ -0,0 +1,12 @@
+package a;
+
+import org.aspectj.lang.annotation.*;
+
+@Aspect
+public class MyAspect {
+  //before(): call(* print(..)) {
+  @Before("call(* print(..))")
+  public void m() {
+    System.out.println("advice running");
+  }
+}
diff --git a/tests/bugs153/pr160674/MyClass.java b/tests/bugs153/pr160674/MyClass.java
new file mode 100644
index 000000000..5909e4169
--- /dev/null
+++ b/tests/bugs153/pr160674/MyClass.java
@@ -0,0 +1,12 @@
+package a;
+
+public class MyClass {
+  public static void main(String []argv) {
+    new MyClass().print("hello");
+    new MyClass().print("world");
+  }
+
+  public void print(String msg) {
+    System.out.println(msg);
+  }
+}
diff --git a/tests/bugs153/pr160674/aop.xml b/tests/bugs153/pr160674/aop.xml
new file mode 100644
index 000000000..ee28eae0f
--- /dev/null
+++ b/tests/bugs153/pr160674/aop.xml
@@ -0,0 +1,7 @@
+<aspectj>
+    <weaver options="-Xset:bcelRepositoryCaching=false -verbose">
+    </weaver>
+    <aspects>
+         <aspect name="a.MyAspect"/> 
+    </aspects>
+</aspectj>
diff --git a/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java b/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java
index e1838f627..88435cf39 100644
--- a/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java
@@ -28,6 +28,7 @@ public class Ajc153Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
   // public void testCFlowXMLAspectLTW_pr149096() { runTest("cflow xml concrete aspect"); }
   // public void testAmbiguousBinding_pr121805() { runTest("ambiguous binding");}
 //  public void testNegatedAnnotationMatchingProblem_pr153464() { runTest("negated annotation matching problem");}
+  public void testTurningOffBcelCaching_pr160674() { runTest("turning off bcel caching");}
   public void testNoIllegalStateExceptionWithGenericInnerAspect_pr156058() { runTest("no IllegalStateException with generic inner aspect"); }
   public void testNoIllegalStateExceptionWithGenericInnerAspect_pr156058_2() { runTest("no IllegalStateException with generic inner aspect - 2"); }
   public void testDeclareMethodAnnotations_pr159143() { runTest("declare method annotations");}
diff --git a/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml b/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml
index a6ddc8ec8..27b7effc1 100644
--- a/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml
+++ b/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml
@@ -117,6 +117,28 @@
       </run>
     </ajc-test>
     
+    <ajc-test dir="bugs153/pr160674" title="turning off bcel caching">
+      <compile files="MyClass.java" options="-1.5"/>
+      <compile files="MyAspect.java" options="-1.5 -Xlint:ignore"/>
+      <run class="a.MyClass" ltw="aop.xml">
+        <stdout>
+          <line text="advice running"/>
+          <line text="hello"/>
+          <line text="advice running"/>
+          <line text="world"/>
+ 	    </stdout>
+ 	    <stderr>
+            <line text="info AspectJ Weaver Version"/>
+            <line text="info register classloader"/>
+            <line text="info using"/>
+            <line text="info register aspect"/>
+            <line text="info [bcelRepositoryCaching=false] AspectJ will not"/>
+            <line text="info processing"/>
+            <line text="info successfully"/>
+ 	    </stderr>
+      </run>
+    </ajc-test>
+    
     <ajc-test dir="bugs153/pr153380/case1" title="pipelining decps">
       <compile files="Ann.java,Base.java,BaseImpl.java,I1.java,Mixin.java,Runner.java,X.aj" options="-1.5">
         <message kind="error" line="1" text="The import java.lang.retention cannot be resolved"/>
diff --git a/weaver/src/org/aspectj/weaver/World.java b/weaver/src/org/aspectj/weaver/World.java
index a670f0eb5..fb5fe193b 100644
--- a/weaver/src/org/aspectj/weaver/World.java
+++ b/weaver/src/org/aspectj/weaver/World.java
@@ -109,6 +109,7 @@ public abstract class World implements Dump.INode {
     private boolean fastDelegateSupportEnabled = isASMAround;
 	private boolean runMinimalMemory = false;
 	private boolean shouldPipelineCompilation = true;
+	protected boolean bcelRepositoryCaching = xsetBCEL_REPOSITORY_CACHING_DEFAULT.equalsIgnoreCase("true");
 	private boolean completeBinaryTypes = false;
 	public boolean forDEBUG_structuralChangesCode = false;
 	public boolean forDEBUG_bridgingCode = false;
@@ -787,10 +788,12 @@ public abstract class World implements Dump.INode {
 	public final static String xsetRUN_MINIMAL_MEMORY ="runMinimalMemory"; // default true
 	public final static String xsetDEBUG_STRUCTURAL_CHANGES_CODE = "debugStructuralChangesCode"; // default false
 	public final static String xsetDEBUG_BRIDGING = "debugBridging"; // default false
+	public final static String xsetBCEL_REPOSITORY_CACHING = "bcelRepositoryCaching";
 	public final static String xsetPIPELINE_COMPILATION = "pipelineCompilation";
 	public final static String xsetPIPELINE_COMPILATION_DEFAULT = "true"; 
 	public final static String xsetCOMPLETE_BINARY_TYPES = "completeBinaryTypes";
 	public final static String xsetCOMPLETE_BINARY_TYPES_DEFAULT = "false"; 
+	public final static String xsetBCEL_REPOSITORY_CACHING_DEFAULT = "true"; 
 	
 	public boolean isInJava5Mode() {
 		return behaveInJava5Way;
@@ -1175,7 +1178,13 @@ public abstract class World implements Dump.INode {
 	        			getMessageHandler().handleMessage(MessageUtil.info("[activateLightweightDelegates=false] Disabling optimization to use lightweight delegates for non-woven types"));
 				}
 				
-				String s = p.getProperty(xsetPIPELINE_COMPILATION,xsetPIPELINE_COMPILATION_DEFAULT);
+				String s = p.getProperty(xsetBCEL_REPOSITORY_CACHING,xsetBCEL_REPOSITORY_CACHING_DEFAULT);
+				bcelRepositoryCaching = s.equalsIgnoreCase("true");
+				if (!bcelRepositoryCaching) {
+					getMessageHandler().handleMessage(MessageUtil.info("[bcelRepositoryCaching=false] AspectJ will not use a bcel cache for class information"));
+				}
+				
+				s = p.getProperty(xsetPIPELINE_COMPILATION,xsetPIPELINE_COMPILATION_DEFAULT);
 				shouldPipelineCompilation = s.equalsIgnoreCase("true");
 
 				s = p.getProperty(xsetCOMPLETE_BINARY_TYPES,xsetCOMPLETE_BINARY_TYPES_DEFAULT);
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java b/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java
index a34454c8d..5dfeba271 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java
@@ -48,6 +48,7 @@ import org.aspectj.apache.bcel.generic.PUTSTATIC;
 import org.aspectj.apache.bcel.generic.Type;
 import org.aspectj.apache.bcel.util.ClassLoaderRepository;
 import org.aspectj.apache.bcel.util.ClassPath;
+import org.aspectj.apache.bcel.util.NonCachingClassLoaderRepository;
 import org.aspectj.apache.bcel.util.Repository;
 import org.aspectj.bridge.IMessageHandler;
 import org.aspectj.weaver.Advice;
@@ -155,10 +156,18 @@ public class BcelWorld extends World implements Repository {
         setMessageHandler(handler);
         setCrossReferenceHandler(xrefHandler);
         // Tell BCEL to use us for resolving any classes
-        delegate = new ClassLoaderRepository(loader);
+        delegate = getClassLoaderRepositoryFor(loader);
         // TODO Alex do we need to call org.aspectj.apache.bcel.Repository.setRepository(delegate);
         // if so, how can that be safe in J2EE ?? (static stuff in Bcel)
     }
+    
+    public Repository getClassLoaderRepositoryFor(ClassLoader loader) {
+    	if (bcelRepositoryCaching) {
+    		return new ClassLoaderRepository(loader);
+    	} else {
+    		return new NonCachingClassLoaderRepository(loader);
+    	}
+    }
 
 	public void addPath (String name) {
 		classPath.addPath(name, this.getMessageHandler());
@@ -355,6 +364,7 @@ public class BcelWorld extends World implements Repository {
             	if (trace.isTraceEnabled()) trace.event("lookupJavaClass",this,new Object[] { name, jc });
                 return jc;
             } catch (ClassNotFoundException e) {
+            	if (trace.isTraceEnabled()) trace.error("Unable to find class '"+name+"' in repository",e);
                 return null;
             }
         }
diff --git a/weaver5/java5-src/org/aspectj/weaver/reflect/Java15AnnotationFinder.java b/weaver5/java5-src/org/aspectj/weaver/reflect/Java15AnnotationFinder.java
index be8d9fd62..c89278b2b 100644
--- a/weaver5/java5-src/org/aspectj/weaver/reflect/Java15AnnotationFinder.java
+++ b/weaver5/java5-src/org/aspectj/weaver/reflect/Java15AnnotationFinder.java
@@ -24,8 +24,8 @@ import java.util.Set;
 import org.aspectj.apache.bcel.classfile.JavaClass;
 import org.aspectj.apache.bcel.classfile.LocalVariable;
 import org.aspectj.apache.bcel.classfile.LocalVariableTable;
+import org.aspectj.apache.bcel.util.NonCachingClassLoaderRepository;
 import org.aspectj.apache.bcel.util.Repository;
-import org.aspectj.apache.bcel.util.ClassLoaderRepository;
 import org.aspectj.weaver.ResolvedType;
 import org.aspectj.weaver.UnresolvedType;
 import org.aspectj.weaver.World;
@@ -45,7 +45,9 @@ public class Java15AnnotationFinder implements AnnotationFinder, ArgNameFinder {
 	}
 	
 	public void setClassLoader(ClassLoader aLoader) {
-		this.bcelRepository = new ClassLoaderRepository(aLoader);
+		// TODO: No easy way to ask the world factory for the right kind of repository so
+		// default to the safe one! (pr160674)
+		this.bcelRepository = new NonCachingClassLoaderRepository(aLoader);
 		this.classLoader = aLoader;		
 	}