Bug 116626 - Load-time weaving - exception from the weaver
Summary: Load-time weaving - exception from the weaver
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: DEVELOPMENT   Edit
Hardware: PC Linux
: P2 critical (vote)
Target Milestone: 1.5.0RC1   Edit
Assignee: Matthew Webster CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 117160 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-11-16 03:54 EST by Misha Kantarovich CLA
Modified: 2005-11-22 00:24 EST (History)
1 user (show)

See Also:


Attachments
Testcase (3.73 KB, application/octet-stream)
2005-11-16 11:37 EST, Matthew Webster CLA
no flags Details
Fix (1.50 KB, application/octet-stream)
2005-11-16 12:03 EST, Matthew Webster CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Misha Kantarovich CLA 2005-11-16 03:54:11 EST
Hi again :-)

I'm trying to use loadtime weaving and getting an exception. Please look at the 
testcase:

Java code:
==========
public class Test<T> {

	Set<T> intsSet;

	public Test() {
		this.intsSet = new HashSet<T>();
	}

	public <T> T[] getObjs(T[] a) {
		return intsSet.toArray(a);
	}

	public static void main(String[] args) {
		System.out.println("AAA :-)");
		new TTT().foo();
	}
}

class TTT {
	public void foo() {
		Test<Object> mt = new Test<Object>();
		Object[] arr = mt.getObjs(new Object[]{});
	}
}

Aspect:
=======
public privileged aspect TestAspect {

      pointcut TestToArray(Test mt) :
                target(mt) &&
                !within(TestAspect);


    Object[] around(Test mt, Object[] objs) :
            TestToArray(mt) &&
            args(objs) &&
            execution(Object[] com.mprv.secsph.Test.getObjs(Object[])) {

        objs = proceed(mt, objs);
        System.out.println("GO Aspects!");
        return objs;
    }
}

aop.xml
=======
<aspectj>
	<aspects>
		<aspect name="com.mprv.secsph.TestAspect"/>
	</aspects>

	<weaver options="-verbose -XlazyTjp -showWeaveInfo">
		<include within="com.mprv.*"/>
	</weaver>
</aspectj>

Program output:
==============
AAA :-)
info weaving 'com/mprv/secsph/TestAspect'
java.lang.NullPointerException
	at 
org.aspectj.weaver.tools.WeavingAdaptor$WeavingClassFileProvider.getBytes
(WeavingAdaptor.java:390)
	at org.aspectj.weaver.tools.WeavingAdaptor.getAtAspectJAspectBytes
(WeavingAdaptor.java:259)
	at org.aspectj.weaver.tools.WeavingAdaptor.weaveClass
(WeavingAdaptor.java:181)
	at org.aspectj.weaver.loadtime.Aj.preProcess(Aj.java:66)
	at org.aspectj.weaver.loadtime.ClassPreProcessorAgentAdapter.transform
(ClassPreProcessorAgentAdapter.java:52)
	at sun.instrument.TransformerManager.transform
(TransformerManager.java:122)
	at sun.instrument.InstrumentationImpl.transform
(InstrumentationImpl.java:155)
	at java.lang.ClassLoader.defineClass1(Native Method)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:620)
	at java.security.SecureClassLoader.defineClass
(SecureClassLoader.java:124)
	at java.net.URLClassLoader.defineClass(URLClassLoader.java:260)
	at java.net.URLClassLoader.access$100(URLClassLoader.java:56)
	at java.net.URLClassLoader$1.run(URLClassLoader.java:195)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.net.URLClassLoader.findClass(URLClassLoader.java:188)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:306)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:268)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:251)
	at java.lang.ClassLoader.loadClassInternal(ClassLoader.java:319)
	at com.mprv.secsph.Test.getObjs(Test.java:1)
	at com.mprv.secsph.TTT.foo(Test.java:34)
	at com.mprv.secsph.Test.main(Test.java:27)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke
(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke
(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:585)
	at com.intellij.rt.execution.application.AppMain.main(AppMain.java:86)
GO Aspects!

====

This exception doesn't happen all the time ... but if you will try a few times, 
you will probably get it. May you can understand what is the problem event 
without running the test case. 

Anyway, I took a look at your code, and have a guess (but it's only the guess) -
My aspect is in the application classpath. Actually it's in the same package, 
so may be you are trying to weave the aspect with itself somehow ...

Thanks!
Misha.
Comment 1 Andrew Clement CLA 2005-11-16 03:56:10 EST
Matthew, can you take a look at recreating this?
Comment 2 Matthew Webster CLA 2005-11-16 11:16:55 EST
This bug is a result of the “fix” to bug 113587. When an aspect is not 
included in the weave it now gets a second chance if it is an @AspectJ aspect 
(see WeavingAdaptor.weaveClass()). However the test in 
BcelWorld.isAnnotationStyleAspect() called from 
shouldWeaveAnnotationStyleAspect() is unreliable because it only looks for the 
org/aspectj/lang/annotation/Aspect annotation which for Java 5 is also added 
to code-style aspects. The trouble happens later in BcelWeaver.weaver() where 
we use the right test so the aspect is not “munged”. This means no new bytes 
are generated and WeavingClassFileProvider.getBytes() throws an NPE. The 
solution is to return the unwoven bytes if there are no woven bytes.

Here are some things I spotted:
1.	The aop.xml is wrong and the TestAspect is no included. It should be 
<include within="com.mprv..*"/> (two dots).
2.	The aspect is loaded and weaving is attempted despite its exclusion.
3.	The application and aspect run correctly.

What is missing from the testcase is how it was built. It has already been 
woven. If it had not been of the aspect had been included correctly the bug 
would not have occurred.
Comment 3 Matthew Webster CLA 2005-11-16 11:37:21 EST
Created attachment 30073 [details]
Testcase

Failing user testcase integrated into harness.
Comment 4 Matthew Webster CLA 2005-11-16 12:03:40 EST
Created attachment 30080 [details]
Fix

WeavingClassFileProvider.getBytes() returns original unwoven bytes if no
weaving takes place.
Comment 5 Matthew Webster CLA 2005-11-16 12:48:00 EST
While this change solves the user's problem I am not happy with the fix. In 
fact I don't think any changes should have been made for bug 113587 in the 
first place. When using annotation-style aspects the Aspects.aspectOf() 
interface should be used not try to fix it up in the run-time.

I have a couple issues with the current implementation:
1. The BcelWorld.isAnnotationStyleAspect() seems to be bogus on Java 5.
2. More seriously the else clause in WeavingAdaptor.weaveClass() ignores 
the "enabled" flag used in shouldWeave() thereby partially invalidating the 
performance improvement in Bug 113511: we create and parse a BCEL JavaClass 
for _every_ excluded class.

I will raise a separate bug to deal with this.
Comment 6 Andrew Clement CLA 2005-11-18 04:07:05 EST
fix integrated, waiting on build.  thanks Matthew.  Oh, and to the original
raiser: you dont need to specify XlazyTjp - its the default now.
Comment 7 Andrew Clement CLA 2005-11-18 07:10:47 EST
Fix available for the NPE.
Comment 8 Ron Bodkin CLA 2005-11-22 00:24:12 EST
*** Bug 117160 has been marked as a duplicate of this bug. ***