Matthews patches from 95529

6 files changed, 70 insertions, 29 deletions

diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
index dca1baab9..3ca6d69cf 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
@@ -66,7 +66,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
     private IWeavingContext weavingContext;
 
     public ClassLoaderWeavingAdaptor(final ClassLoader loader, IWeavingContext wContext) {
-        super(null);
+    	this.weavingContext = wContext;
     }
 
     void initialize(final ClassLoader loader, IWeavingContext wContext) {
@@ -132,7 +132,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
      */
     private void registerDefinitions(final BcelWeaver weaver, final ClassLoader loader) {
         try {
-            MessageUtil.info(messageHandler, "register classloader " + ((loader!=null)?loader.getClass().getName()+"@"+loader.hashCode():"null"));
+            MessageUtil.info(messageHandler, "register classloader " + getClassLoaderName(loader));
             //TODO av underoptimized: we will parse each XML once per CL that see it
             List definitions = new ArrayList();
 
@@ -172,6 +172,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
                 registerDump(weaver, loader, definitions);
             } else {
                 enabled = false;// will allow very fast skip in shouldWeave()
+        		info("no configuration found. Disabling weaver for class loader " + getClassLoaderName(loader));
             }
         } catch (Exception e) {
             weaver.getWorld().getMessageHandler().handleMessage(
@@ -180,6 +181,10 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         }
     }
 
+    private String getClassLoaderName (ClassLoader loader) {
+    	return weavingContext.getClassLoaderName();
+   	}
+    
     /**
      * Configure the weaver according to the option directives
      * TODO av - don't know if it is that good to reuse, since we only allow a small subset of options in LTW
@@ -195,7 +200,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
             allOptions.append(definition.getWeaverOptions()).append(' ');
         }
 
-        Options.WeaverOption weaverOption = Options.parse(allOptions.toString(), loader);
+        Options.WeaverOption weaverOption = Options.parse(allOptions.toString(), loader, messageHandler);
 
         // configure the weaver and world
         // AV - code duplicates AspectJBuilder.initWorldAndWeaver()
@@ -297,13 +302,9 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
             for (Iterator aspects = definition.getAspectClassNames().iterator(); aspects.hasNext();) {
                 String aspectClassName = (String) aspects.next();
                 if (acceptAspect(aspectClassName)) {
+                	info("register aspect " + aspectClassName);
                     ResolvedType aspect = weaver.addLibraryAspect(aspectClassName);
-                    if (aspect.isAbstract()) {
-                        // this is a warning
-                        weaver.getWorld().getMessageHandler().handleMessage(
-                                new Message("Abstract aspect registered in aop.xml, use a <concrete-aspect> element instead", IMessage.WARNING, null, null)
-                        );
-                    }
+
                     //generate key for SC
                     if(namespace==null){
                     	namespace=new StringBuffer(aspectClassName);
@@ -323,9 +324,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
                 if (acceptAspect(concreteAspect.name)) {
                     ConcreteAspectCodeGen gen = new ConcreteAspectCodeGen(concreteAspect, weaver.getWorld());
                     if (!gen.validate()) {
-                        weaver.getWorld().getMessageHandler().handleMessage(
-                                new Message("Concrete-aspect '"+concreteAspect.name+"' could not be registered", IMessage.ERROR, null, null)
-                        );
+                        error("Concrete-aspect '"+concreteAspect.name+"' could not be registered");
                         break;
                     }
                     this.generatedClassHandler.acceptClass(
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java b/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java
index 7110e0c54..092eddb7e 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/DefaultWeavingContext.java
@@ -22,7 +22,7 @@ import java.util.Enumeration;
  */
 public class DefaultWeavingContext implements IWeavingContext {
 	
-	private ClassLoader loader;
+	protected ClassLoader loader;
 	
 	public DefaultWeavingContext(){
 		loader = getClass().getClassLoader();
@@ -51,4 +51,11 @@ public class DefaultWeavingContext implements IWeavingContext {
 		return null;
 	}
 
+	/**
+	 * @return classname@hashcode
+	 */
+	public String getClassLoaderName() {
+    	return ((loader!=null)?loader.getClass().getName()+"@"+loader.hashCode():"null");
+	}
+
 }
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/IWeavingContext.java b/loadtime/src/org/aspectj/weaver/loadtime/IWeavingContext.java
index 285fc81dc..f634cabc0 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/IWeavingContext.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/IWeavingContext.java
@@ -41,5 +41,12 @@ public interface IWeavingContext {
 	 * @return
 	 */
 	public String getBundleIdFromURL(URL url);
+	
+	/**
+	 * In an environment with multiple class loaders allows each to be
+	 * identified using something safer and than toString
+	 * @return name of the associated class loader
+	 */
+	public String getClassLoaderName ();
 
 }
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/Options.java b/loadtime/src/org/aspectj/weaver/loadtime/Options.java
index 5bc469470..d96f45cb1 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/Options.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/Options.java
@@ -45,16 +45,16 @@ public class Options {
     private static final String OPTIONVALUED_Xlint = "-Xlint:";
 
 
-    public static WeaverOption parse(String options, ClassLoader laoder) {
+    public static WeaverOption parse(String options, ClassLoader laoder, IMessageHandler imh) {
+        WeaverOption weaverOption = new WeaverOption(imh);
+
         if (LangUtil.isEmpty(options)) {
-            return new WeaverOption();
+            return weaverOption;
         }
         // the first option wins
         List flags = LangUtil.anySplit(options, " ");
         Collections.reverse(flags);
 
-        WeaverOption weaverOption = new WeaverOption();
-
         // do a first round on the message handler since it will report the options themselves
         for (Iterator iterator = flags.iterator(); iterator.hasNext();) {
             String arg = (String) iterator.next();
@@ -152,8 +152,9 @@ public class Options {
         String lint;
         String lintFile;
 
-        public WeaverOption() {
-            messageHandler = new DefaultMessageHandler();//default
+        public WeaverOption(IMessageHandler imh) {
+//            messageHandler = new DefaultMessageHandler();//default
+            this.messageHandler = imh;
         }
     }
 }
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
index 8c7035b5e..4fe476eb1 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
@@ -34,7 +34,8 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W
 	public static final String WEAVING_ASPECT_PATH = "aj.aspect.path"; 
 	
 	private URL[] aspectURLs;
-	private WeavingAdaptor adaptor; 
+	private WeavingAdaptor adaptor;
+	private boolean initializingAdaptor;
 	private Map generatedClasses = new HashMap(); /* String -> byte[] */ 
 
 	/*
@@ -99,15 +100,30 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W
 	 */
 	protected Class defineClass(String name, byte[] b, CodeSource cs) throws IOException {
 //		System.err.println("? WeavingURLClassLoader.defineClass(" + name + ", [" + b.length + "])");
-		
-		/* Need to defer creation because of possible recursion during constructor execution */
-		if (adaptor == null) {
-			ClassLoaderWeavingAdaptor clwAdaptor = new ClassLoaderWeavingAdaptor(this,null);
-			clwAdaptor.initialize(this,null);
-			adaptor = clwAdaptor;
+
+		/* Avoid recursion during adaptor initialization */
+		if (!initializingAdaptor) {
+			
+			/* Need to defer creation because of possible recursion during constructor execution */
+			if (adaptor == null && !initializingAdaptor) {
+				DefaultWeavingContext weavingContext = new DefaultWeavingContext (this) {
+
+					/* Ensures consistent LTW messages for testing */
+					public String getClassLoaderName() {
+						return loader.getClass().getName();
+					}
+					
+				};
+				
+				ClassLoaderWeavingAdaptor clwAdaptor = new ClassLoaderWeavingAdaptor(this,weavingContext);
+				initializingAdaptor = true;
+				clwAdaptor.initialize(this,weavingContext);
+				initializingAdaptor = false;
+				adaptor = clwAdaptor;
+			}
+			
+			b = adaptor.weaveClass(name,b);
 		}
-		
-		b = adaptor.weaveClass(name,b);
 		return super.defineClass(name, b, cs);
 	}
 
diff --git a/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingURLClassLoaderTest.java b/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingURLClassLoaderTest.java
index fe3139314..093a0125a 100644
--- a/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingURLClassLoaderTest.java
+++ b/loadtime/testsrc/org/aspectj/weaver/loadtime/WeavingURLClassLoaderTest.java
@@ -69,6 +69,11 @@ public class WeavingURLClassLoaderTest extends TestCase {
 		}
 	}
 
+	/*
+	 * We won't get an exception because the aspect path is empty and there is 
+	 * no aop.xml file so the weaver will be disabled and no reweaving will 
+	 * take place 
+	 */
 	public void testLoadWovenClass () {
 		setSystemProperty(WeavingURLClassLoader.WEAVING_ASPECT_PATH,"");
 		setSystemProperty(WeavingURLClassLoader.WEAVING_CLASS_PATH,WOVEN_JAR);
@@ -83,6 +88,9 @@ public class WeavingURLClassLoaderTest extends TestCase {
 		}
 	}
 
+	/*
+	 * We get an exception because the class was not built reweavable
+	 */
 	public void testWeaveWovenClass () {
 		setSystemProperty(WeavingURLClassLoader.WEAVING_ASPECT_PATH,ADVICE_ASPECTS);
 		setSystemProperty(WeavingURLClassLoader.WEAVING_CLASS_PATH,ADVICE_ASPECTS + File.pathSeparator + WOVEN_JAR);
@@ -345,6 +353,7 @@ public class WeavingURLClassLoaderTest extends TestCase {
 			fail("Expecting java.lang.NoClassDefFoundError");
 		}
 		catch (Exception ex) {
+			assertTrue("Expecting java.lang.NoClassDefFoundError but caught " + ex,ex.getMessage().contains("java.lang.NoClassDefFoundError"));
 		}
 	}
 
@@ -463,7 +472,9 @@ public class WeavingURLClassLoaderTest extends TestCase {
 			method.invoke(null,params);
 		}
 		catch (InvocationTargetException ex) {
-			throw new RuntimeException(ex.getTargetException().toString());
+			Throwable targetException = ex.getTargetException();
+			if (targetException instanceof RuntimeException) throw (RuntimeException)ex.getTargetException();
+			else throw new RuntimeException(ex.getTargetException().toString());
 		}
 		catch (Exception ex) {
 			throw new RuntimeException(ex.toString());