Bug 158957 "NPE in LTW with RMI dynamic proxies w/ pointcut reuse" (restore catch...

diff --git a/loadtime/src/org/aspectj/weaver/loadtime/Aj.java b/loadtime/src/org/aspectj/weaver/loadtime/Aj.java
index 2f775a0b107ee8dad55a8c983c0e77122a985fff..e4155d833e8a71988e07751d9a2041d8a71afa43 100644 (file)
--- a/loadtime/src/org/aspectj/weaver/loadtime/Aj.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/Aj.java
@@ -78,12 +78,13 @@ public class Aj implements ClassPreProcessor {
                        if (trace.isTraceEnabled()) trace.exit("preProcess",newBytes);
                 return newBytes;
                        }
-        } catch (Exception ex) {
-               trace.error("preProcess",ex);
+        
+        /* Don't like to do this but JVMTI swallows all exceptions */
+        } catch (Throwable th) {
+               trace.error("preProcess",th);
             //FIXME AV wondering if we should have the option to fail (throw runtime exception) here
             // would make sense at least in test f.e. see TestHelper.handleMessage()
-            ex.printStackTrace();
-               if (trace.isTraceEnabled()) trace.exit("preProcess",ex);
+               if (trace.isTraceEnabled()) trace.exit("preProcess",th);
             return bytes;
         }
     }

diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
index 6d814ef95ac8c34c57e912850cf3fe50b193680a..0a210f0e31bfb871ad3a48f8c2425099736277b4 100644 (file)
--- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
@@ -98,9 +98,9 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
     }
 
     protected void initialize (final ClassLoader classLoader, IWeavingContext context) {
-        //super(null);// at this stage we don't have yet a generatedClassHandler to define to the VM the closures
        if (initialized) return;
 
+       boolean success = true;
        if (trace.isTraceEnabled()) trace.enter("initialize",this,new Object[] { classLoader, context });
 
        this.weavingContext = context;
@@ -131,8 +131,9 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         };
 
         List definitions = parseDefinitions(classLoader);
-        if (!isEnabled()) {
-               if (trace.isTraceEnabled()) trace.exit("initialize",false);
+        if (definitions.isEmpty()) {
+               disable(); // TODO maw Needed to ensure messages are flushed
+               if (trace.isTraceEnabled()) trace.exit("initialize",definitions);
                return;
         }
         
@@ -144,26 +145,25 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
                     }
                 }
         );
-//            //TODO this AJ code will call
-//            //org.aspectj.apache.bcel.Repository.setRepository(this);
-//            //ie set some static things
-//            //==> bogus as Bcel is expected to be
-//            org.aspectj.apache.bcel.Repository.setRepository(new ClassLoaderRepository(loader));
 
         weaver = new BcelWeaver(bcelWorld);
 
         // register the definitions
-        registerDefinitions(weaver, classLoader, definitions);
-        if (isEnabled()) {
-
-            //bcelWorld.setResolutionLoader(loader.getParent());//(ClassLoader)null);//
+        success = registerDefinitions(weaver, classLoader, definitions);
+        if (success) {
             
             // after adding aspects
             weaver.prepareForWeave();
-            boolean success = weaveAndDefineConceteAspects();
-            if (!success) disable();
+            
+            enable(); // TODO maw Needed to ensure messages are flushed
+            success = weaveAndDefineConceteAspects();
+        }
+
+        if (success) {
+            enable();
         }
         else {
+               disable();
                bcelWorld = null;
                weaver = null;
         }
@@ -179,6 +179,8 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
      * @param loader
      */
     private List parseDefinitions(final ClassLoader loader) {
+        if (trace.isTraceEnabled()) trace.enter("parseDefinitions",this,loader);
+
         List definitions = new ArrayList();
        try {
             info("register classloader " + getClassLoaderName(loader));
@@ -194,7 +196,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
             }
 
             String resourcePath = System.getProperty("org.aspectj.weaver.loadtime.configuration",AOP_XML);
-            trace.event("parseDefinitions",this,resourcePath);
+            if (trace.isTraceEnabled()) trace.event("parseDefinitions",this,resourcePath);
                StringTokenizer st = new StringTokenizer(resourcePath,";");
 
                while(st.hasMoreTokens()){
@@ -204,7 +206,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
                        Set seenBefore = new HashSet();
                        while (xmls.hasMoreElements()) {
                            URL xml = (URL) xmls.nextElement();
-                   trace.event("parseDefinitions",this,xml);
+                           if (trace.isTraceEnabled()) trace.event("parseDefinitions",this,xml);
                            if (!seenBefore.contains(xml)) {
                                info("using configuration " + weavingContext.getFile(xml));
                                definitions.add(DocumentParser.parse(xml));
@@ -216,28 +218,36 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
                        }
                }
                if (definitions.isEmpty()) {
-                disable();// will allow very fast skip in shouldWeave()
                        info("no configuration found. Disabling weaver for class loader " + getClassLoaderName(loader));
                }
         } catch (Exception e) {
-            disable();// will allow very fast skip in shouldWeave()
+               definitions.clear();
             warn("parse definitions failed",e);
         }
+        
+        if (trace.isTraceEnabled()) trace.exit("parseDefinitions",definitions);
                return definitions;
     }
         
-    private void registerDefinitions(final BcelWeaver weaver, final ClassLoader loader, List definitions) {
-       try {
+    private boolean registerDefinitions(final BcelWeaver weaver, final ClassLoader loader, List definitions) {
+        if (trace.isTraceEnabled()) trace.enter("registerDefinitions",this,definitions);
+        boolean success = true;
+
+        try {
             registerOptions(weaver, loader, definitions);
             registerAspectExclude(weaver, loader, definitions);
             registerAspectInclude(weaver, loader, definitions);
-            registerAspects(weaver, loader, definitions);
+            success = registerAspects(weaver, loader, definitions);
             registerIncludeExclude(weaver, loader, definitions);
             registerDump(weaver, loader, definitions);
-        } catch (Exception e) {
-            disable();// will allow very fast skip in shouldWeave()
-            warn("register definition failed",(e instanceof AbortException)?null:e);
+        } catch (Exception ex) {
+            trace.error("register definition failed",ex);
+            success = false;
+            warn("register definition failed",(ex instanceof AbortException)?null:ex);
         }
+        
+        if (trace.isTraceEnabled()) trace.exit("registerDefinitions",success);
+        return success;
     }
 
     private String getClassLoaderName (ClassLoader loader) {
@@ -370,7 +380,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
      * @param loader
      * @param definitions
      */
-    private void registerAspects(final BcelWeaver weaver, final ClassLoader loader, final List definitions) {
+    private boolean 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;
        
@@ -435,17 +445,17 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         
         /* 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 */
         else if (namespace == null) {
-               disable();
+               success = false;
                info("no aspects registered. Disabling weaver for class loader " + getClassLoaderName(loader));
         }
 
-        if (trace.isTraceEnabled()) trace.exit("registerAspects",isEnabled());
+        if (trace.isTraceEnabled()) trace.exit("registerAspects",success);
+        return success;
     }
 
     private boolean weaveAndDefineConceteAspects () {

diff --git a/tests/bugs153/pr158957/HelloWorld.java b/tests/bugs153/pr158957/HelloWorld.java
new file mode 100644 (file)
index 0000000..8f62298
--- /dev/null
+++ b/tests/bugs153/pr158957/HelloWorld.java
@@ -0,0 +1,11 @@
+public class HelloWorld {
+
+       public void println () {
+               System.out.println("Hello World!");
+       }
+       
+       public static void main (String[] args) throws Exception {
+               new HelloWorld().println();
+       }
+       
+}
\ No newline at end of file

diff --git a/tests/bugs153/pr158957/Missing.java b/tests/bugs153/pr158957/Missing.java
new file mode 100644 (file)
index 0000000..a61ca7b
--- /dev/null
+++ b/tests/bugs153/pr158957/Missing.java
@@ -0,0 +1,3 @@
+public interface Missing {
+       public void doIt ();
+}
\ No newline at end of file

diff --git a/tests/bugs153/pr158957/PointcutLibrary.aj b/tests/bugs153/pr158957/PointcutLibrary.aj
new file mode 100644 (file)
index 0000000..ffda35e
--- /dev/null
+++ b/tests/bugs153/pr158957/PointcutLibrary.aj
@@ -0,0 +1,9 @@
+public aspect PointcutLibrary {
+
+       public static pointcut doIt () :
+               execution(public void doIt()) && this(Missing);
+       
+       public static pointcut println () :
+               execution(public void println(..));
+       
+}
\ No newline at end of file

diff --git a/tests/bugs153/pr158957/Tracing.aj b/tests/bugs153/pr158957/Tracing.aj
new file mode 100644 (file)
index 0000000..5a974ad
--- /dev/null
+++ b/tests/bugs153/pr158957/Tracing.aj
@@ -0,0 +1,6 @@
+public aspect Tracing {
+       
+       before () : PointcutLibrary.println() {
+               System.out.println("? " + thisJoinPointStaticPart.getSignature().getName());
+       }
+}
\ No newline at end of file

diff --git a/tests/bugs153/pr158957/ant.xml b/tests/bugs153/pr158957/ant.xml
new file mode 100644 (file)
index 0000000..c2de686
--- /dev/null
+++ b/tests/bugs153/pr158957/ant.xml
@@ -0,0 +1,27 @@
+<!-- ajc-ant script, not to be used from Ant commant line - see AntSpec -->
+<project name="ltw">
+
+    <!-- using this we can debug the forked VM -->
+    <property
+        name="jdwp"
+        value="-Xdebug -Xnoagent -Djava.compiler=NONE -Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=5005"/>
+       <property name="aj.bootpath" refid="aj.path"/>
+
+    <target name="NPE with LTW, pointcut library and missing aspect dependency">
+        <java fork="yes" classname="HelloWorld" failonerror="yes">
+            <classpath refid="aj.path"/>
+            <classpath>
+               <!--<pathelement path="${aj.sandbox}/hello.jar:${aj.sandbox}/tracing.jar:${aj.sandbox}/missing.jar"/>-->
+               <pathelement path="${aj.sandbox}/hello.jar:${aj.sandbox}/tracing.jar"/>
+            </classpath>
+            <jvmarg value="-Daj.weaving.verbose=true"/>
+            <jvmarg value="-Dorg.aspectj.weaver.showWeaveInfo=true"/>
+            <jvmarg value="-javaagent:${aj.root}/lib/test/loadtime5.jar"/>
+            <jvmarg value="-Dorg.aspectj.tracing.enabled=true"/>
+            <jvmarg value="-Dorg.aspectj.tracing.factory=default"/>
+            <jvmarg value="-Dorg.aspectj.tracing.messages=true"/>
+<!--            <jvmarg line="${jdwp}"/>-->
+        </java>
+    </target>
+
+</project>

diff --git a/tests/bugs153/pr158957/aop.xml b/tests/bugs153/pr158957/aop.xml
new file mode 100644 (file)
index 0000000..fec692f
--- /dev/null
+++ b/tests/bugs153/pr158957/aop.xml
@@ -0,0 +1,9 @@
+<aspectj>
+       <aspects>
+           <aspect name="PointcutLibrary"/>
+           <aspect name="Tracing"/>
+       </aspects>
+       
+       <weaver options="-1.5"/>
+</aspectj>
+

diff --git a/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java b/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java
index caef4be71b2711ebdfeace7df37b8de9bd37bc2e..3aa129227dd4897539c478aea3ee13ee8a417d8a 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc153/Ajc153Tests.java
@@ -150,6 +150,10 @@ public class Ajc153Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
   public void testWeaveConcreteSubaspectWithCflow_pr132080() {
          runTest("Weave concrete sub-aspect with cflow");
   }
+
+  public void testNPEWithLTWPointcutLibraryAndMissingAspectDependency_pr158957 () {
+         runTest("NPE with LTW, pointcut library and missing aspect dependency");
+  }
   
   public void testNoInvalidAbsoluteTypeNameWarning_pr156904_1() {runTest("ensure no invalidAbsoluteTypeName when do match - 1");}
   public void testNoInvalidAbsoluteTypeNameWarning_pr156904_2() {runTest("ensure no invalidAbsoluteTypeName when do match - 2");}

diff --git a/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml b/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml
index 9b64b631fe536dddd2c78963e55ea3b872d32e42..7e3f4e88885044923072e3221defc284b00bc4bc 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml
+++ b/tests/src/org/aspectj/systemtest/ajc153/ajc153.xml
@@ -542,6 +542,27 @@
                <line text="Hello World!"/>
                </stdout>
         </ant>
+    </ajc-test>
+
+       <ajc-test dir="bugs153/pr158957" title="NPE with LTW, pointcut library and missing aspect dependency" keywords="ltw">
+        <compile files="HelloWorld.java" options="-outjar hello.jar"/>
+        <compile files="Missing.java" options="-outjar missing.jar"/>
+        <compile files="Tracing.aj, PointcutLibrary.aj" options="-Xlint:ignore -1.5 -outxml -outjar tracing.jar" classpath="hello.jar,missing.jar"/>
+<!--        
+               <run class="HelloWorld" ltw="aop.xml">
+               <stdout>
+             <line text="? main"/>
+             <line text="Hello World!"/>
+               </stdout>
+        </run>
+-->        
+               <ant file="ant.xml" target="NPE with LTW, pointcut library and missing aspect dependency" verbose="true">
+               <stdout>
+             <line text="? main"/>
+             <line text="Hello World!"/>
+               </stdout>
+        </ant>
+
     </ajc-test>
 
        <ajc-test dir="bugs153/pr132080"
@@ -618,5 +639,25 @@
                <run class="TestMain" ltw="aop-pr149096.xml"/>
     </ajc-test>
     
+       <ajc-test dir="bugs153/pr158957" title="NPE with LTW, pointcut library and missing aspect dependency" keywords="ltw">
+        <compile files="HelloWorld.java" options="-outjar hello.jar"/>
+        <compile files="Missing.java" options="-outjar missing.jar"/>
+        <compile files="Tracing.aj, PointcutLibrary.aj" options="-Xlint:ignore -1.5 -outxml -outjar tracing.jar" classpath="hello.jar,missing.jar"/>
+<!--        
+               <run class="HelloWorld" ltw="aop.xml">
+               <stdout>
+             <line text="? main"/>
+             <line text="Hello World!"/>
+               </stdout>
+        </run>
+-->        
+               <ant file="ant.xml" target="NPE with LTW, pointcut library and missing aspect dependency" verbose="true">
+               <stdout>
+             <line text="? println"/>
+             <line text="Hello World!"/>
+               </stdout>
+        </ant>
+
+    </ajc-test>
 
 </suite>
\ No newline at end of file

diff --git a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
index 7d52c3afa698689d3e51c7de7e76b2c9f29ddffd..0d0420132790d300a53403bae6f1755243f2b215 100644 (file)
--- a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
+++ b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
@@ -74,7 +74,7 @@ public class WeavingAdaptor implements IMessageContext {
        public static final String SHOW_WEAVE_INFO_PROPERTY = "org.aspectj.weaver.showWeaveInfo"; 
        public static final String TRACE_MESSAGES_PROPERTY = "org.aspectj.tracing.messages";
 
-       private boolean enabled = true;
+       private boolean enabled = false;
        protected boolean verbose = getVerbose();
        protected BcelWorld bcelWorld;
        protected BcelWeaver weaver;
@@ -169,6 +169,8 @@ public class WeavingAdaptor implements IMessageContext {
 
                weaver = new BcelWeaver(bcelWorld);
                registerAspectLibraries(aspectPath);
+               
+               enabled = true;
        }
 
        protected void createMessageHandler() {
@@ -201,6 +203,11 @@ public class WeavingAdaptor implements IMessageContext {
                if (trace.isTraceEnabled()) trace.exit("disable");
        }
        
+       protected void enable () {
+               enabled = true;
+               messageHolder.flushMessages();
+       }
+       
        protected boolean isEnabled () {
                return enabled;
        }
@@ -227,6 +234,8 @@ public class WeavingAdaptor implements IMessageContext {
      * @exception IOException weave failed
         */
        public byte[] weaveClass (String name, byte[] bytes) throws IOException {
+               if (trace.isTraceEnabled()) trace.enter("weaveClass",this,new Object[] {name, bytes});
+
                if (enabled) {
                        try {
                                delegateForCurrentClass=null; 
@@ -258,6 +267,7 @@ public class WeavingAdaptor implements IMessageContext {
                        }
                }
 
+               if (trace.isTraceEnabled()) trace.exit("weaveClass",bytes);
         return bytes;
        }