From: mwebster Date: Thu, 24 Aug 2006 17:32:00 +0000 (+0000) Subject: Bug 129525 "Don't Dump Bytecodes to Syserr in LTW" (don't throw AbortException in... X-Git-Tag: BEFORE_133532~80 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=04fa1dcce12934cc5cfe1e5e8b66523c10e93672;p=aspectj.git Bug 129525 "Don't Dump Bytecodes to Syserr in LTW" (don't throw AbortException in LTW message handler) --- diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java index 2e53d0cfa..7e2edb5c0 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java @@ -355,6 +355,8 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { */ private void registerAspects(final BcelWeaver weaver, final ClassLoader loader, final List definitions) { if (trace.isTraceEnabled()) trace.enter("registerAspects",this, new Object[] { weaver, loader, definitions} ); + boolean success = true; + //TODO: the exclude aspect allow to exclude aspect defined upper in the CL hierarchy - is it what we want ?? // if not, review the getResource so that we track which resource is defined by which CL @@ -393,6 +395,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { ConcreteAspectCodeGen gen = new ConcreteAspectCodeGen(concreteAspect, weaver.getWorld()); if (!gen.validate()) { error("Concrete-aspect '"+concreteAspect.name+"' could not be registered"); + success = false; break; } this.generatedClassHandler.acceptClass( @@ -412,8 +415,14 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor { } // System.out.println("ClassLoaderWeavingAdaptor.registerAspects() classloader=" + weavingContext.getClassLoaderName() + ", namespace=" + namespace); + /* We couldn't register one or more aspects so disable the adaptor */ + if (!success) { + disable(); + warn("failure(s) registering aspects. Disabling weaver for class loader " + getClassLoaderName(loader)); + } + /* We didn't register any aspects so disable the adaptor */ - if (namespace == null) { + else if (namespace == null) { disable(); info("no aspects registered. Disabling weaver for class loader " + getClassLoaderName(loader)); } diff --git a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java index 0dc1506df..fca4a23a5 100644 --- a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java +++ b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java @@ -24,7 +24,10 @@ import java.util.List; import java.util.Map; import java.util.StringTokenizer; +import org.aspectj.bridge.AbortException; import org.aspectj.weaver.ExtensibleURLClassLoader; +import org.aspectj.weaver.tools.Trace; +import org.aspectj.weaver.tools.TraceFactory; import org.aspectj.weaver.tools.WeavingAdaptor; import org.aspectj.weaver.tools.WeavingClassLoader; @@ -37,6 +40,8 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W private WeavingAdaptor adaptor; private boolean initializingAdaptor; private Map generatedClasses = new HashMap(); /* String -> byte[] */ + + private static Trace trace = TraceFactory.getTraceFactory().getTrace(WeavingURLClassLoader.class); /* * This constructor is needed when using "-Djava.system.class.loader". @@ -48,7 +53,9 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W public WeavingURLClassLoader (URL[] urls, ClassLoader parent) { super(urls,parent); + if (trace.isTraceEnabled()) trace.enter("",this,new Object[] { urls, parent }); // System.out.println("WeavingURLClassLoader.WeavingURLClassLoader()"); + if (trace.isTraceEnabled()) trace.exit(""); } public WeavingURLClassLoader (URL[] classURLs, URL[] aspectURLs, ClassLoader parent) { @@ -106,6 +113,7 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W * Override to weave class using WeavingAdaptor */ protected Class defineClass(String name, byte[] b, CodeSource cs) throws IOException { + if (trace.isTraceEnabled()) trace.enter("defineClass",this,new Object[] { name, b, cs }); // System.err.println("? WeavingURLClassLoader.defineClass(" + name + ", [" + b.length + "])"); /* Avoid recursion during adaptor initialization */ @@ -116,9 +124,20 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W createAdaptor(); } - b = adaptor.weaveClass(name,b); + try { + b = adaptor.weaveClass(name,b); + } + catch (AbortException ex) { + trace.error("defineClass",ex); + throw ex; + } + catch (Throwable th) { + trace.error("defineClass",th); + } } - return super.defineClass(name, b, cs); + Class clazz = super.defineClass(name, b, cs); + if (trace.isTraceEnabled()) trace.exit("defineClass",clazz); + return clazz; } private void createAdaptor () { diff --git a/tests/ltw/aop-missingaspect.xml b/tests/ltw/aop-missingaspect.xml new file mode 100644 index 000000000..27235c38b --- /dev/null +++ b/tests/ltw/aop-missingaspect.xml @@ -0,0 +1,5 @@ + + + + + \ No newline at end of file diff --git a/tests/ltw/pakkage/EmptyAspect.aj b/tests/ltw/pakkage/EmptyAspect.aj new file mode 100644 index 000000000..e24bad3e8 --- /dev/null +++ b/tests/ltw/pakkage/EmptyAspect.aj @@ -0,0 +1,3 @@ +public aspect EmptyAspect { + +} \ No newline at end of file diff --git a/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml b/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml index 812d15830..3e7b08082 100644 --- a/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml +++ b/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml @@ -483,7 +483,8 @@ - + + @@ -497,7 +498,8 @@ - + + diff --git a/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java b/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java index b366d6798..9ae265bd1 100644 --- a/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java +++ b/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java @@ -66,6 +66,10 @@ public class Ajc153Tests extends org.aspectj.testing.XMLBasedAjcTestCase { public void testDuplicateJVMTIAgents_pr151938() {runTest("Duplicate JVMTI agents");}; public void testLTWWorldWithAnnotationMatching_pr153572() { runTest("LTWWorld with annotation matching");} + public void testReweavableAspectNotRegistered_pr129525 () { + runTest("reweavableAspectNotRegistered error"); + } + ///////////////////////////////////////// public static Test suite() { return XMLBasedAjcTestCase.loadSuite(Ajc153Tests.class); diff --git a/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml b/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml index f7586c342..b1910ee63 100644 --- a/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml +++ b/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml @@ -333,4 +333,20 @@ + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/src/org/aspectj/systemtest/tracing/tracing.xml b/tests/src/org/aspectj/systemtest/tracing/tracing.xml index 6a5d4881e..bed65f12d 100644 --- a/tests/src/org/aspectj/systemtest/tracing/tracing.xml +++ b/tests/src/org/aspectj/systemtest/tracing/tracing.xml @@ -52,8 +52,10 @@ - - + + + + diff --git a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java index b1adcfb8f..26244e2a0 100644 --- a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java +++ b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java @@ -80,6 +80,7 @@ public class WeavingAdaptor implements IMessageContext { protected BcelWeaver weaver; private IMessageHandler messageHandler; private WeavingAdaptorMessageHandler messageHolder; + private boolean abortOnError = false; protected GeneratedClassHandler generatedClassHandler; protected Map generatedClasses = new HashMap(); /* String -> UnwovenClassFile */ protected BcelObjectType delegateForCurrentClass; // lazily initialized, should be used to prevent parsing bytecode multiple times @@ -153,6 +154,7 @@ public class WeavingAdaptor implements IMessageContext { } private void init(List classPath, List aspectPath) { + abortOnError = true; createMessageHandler(); info("using classpath: " + classPath); @@ -191,8 +193,12 @@ public class WeavingAdaptor implements IMessageContext { } protected void disable () { + if (trace.isTraceEnabled()) trace.enter("disable",this); + enabled = false; messageHolder.flushMessages(); + + if (trace.isTraceEnabled()) trace.exit("disable"); } protected boolean isEnabled () { @@ -469,7 +475,7 @@ public class WeavingAdaptor implements IMessageContext { if (traceMessages) traceMessage(message); if (accumulating) { boolean result = addMessage(message); - if (0 <= message.getKind().compareTo(IMessage.ERROR)) { + if (abortOnError && 0 <= message.getKind().compareTo(IMessage.ERROR)) { throw new AbortException(message); } return result; @@ -553,7 +559,7 @@ public class WeavingAdaptor implements IMessageContext { public boolean handleMessage(IMessage message) throws AbortException { boolean result = super.handleMessage(message); - if (0 <= message.getKind().compareTo(failKind)) { + if (abortOnError && 0 <= message.getKind().compareTo(failKind)) { throw new AbortException(message); } return true;