Bug 49657 - Weaver results in rt error if affected base class not exposed
Summary: Weaver results in rt error if affected base class not exposed
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: 1.1.1   Edit
Hardware: PC Windows 2000
: P3 normal (vote)
Target Milestone: 1.2.1   Edit
Assignee: Andrew Clement CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-01-07 17:27 EST by Ron Bodkin CLA
Modified: 2004-10-21 04:32 EDT (History)
0 users

See Also:


Attachments
test case (1.70 KB, application/x-zip-compressed)
2004-01-07 17:28 EST, Ron Bodkin CLA
no flags Details
revised test case that exposes the actual bug (1.63 KB, application/x-zip-compressed)
2004-01-21 14:53 EST, Ron Bodkin CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ron Bodkin CLA 2004-01-07 17:27:35 EST
I was weaving a 3rd party library and wrote an aspect that should affect both a 
base class and a derived class, but inadvertently wove only the derived class.  
This resulted in the following error. I would like the weaver to catch this 
kind of error at weave (link) time, rather than creating code that results in a 
rt error... I get this error in 1.1.1 final and in a fairly recent version from 
CVS HEAD:

java.lang.AbstractMethodError
	at ajee.logging.LogManager.ajc$interMethodDispatch1
$ajee_logging_LogManager$ajee_logging_Loggable$getLogger(LogManager.java)
	at 
ajee.tracing.ExecutionTracer.ajc$before$ajee_tracing_ExecutionTracer$926
(ExecutionTracer.java:62)
	at library.Derived.<init>(Derived.java:46)

This line is a call to super();

I've attached a small example that reproduces another error that I think is 
related:
C:\devel\test\ctors\twoPass>build

C:\devel\test\ctors\twoPass>call ajc -outjar fullBase.jar sample\Base.java sampl
e\Derived.java
Exception in thread "main" java.lang.ExceptionInInitializerError
        at sample.Derived.<init>(Derived.java:6)
        at sample.Derived.main(Derived.java:13)
Caused by: org.aspectj.lang.NoAspectBoundException
        at sample.Trace.aspectOf(Trace.aj)
        at sample.Trace.<init>(Trace.aj:5)
        at sample.Trace.ajc$postClinit(Trace.aj)
        at sample.Trace.<clinit>(Trace.aj:5)
        ... 2 more
Comment 1 Ron Bodkin CLA 2004-01-07 17:28:08 EST
Created attachment 7354 [details]
test case
Comment 2 Matthew Webster CLA 2004-01-08 10:21:58 EST
I can reproduce the failure in the testcase however the NoAspectBoundException 
is because the Trace aspect is advising itself (it is also in the sample 
package). If a "!within(Trace)" is used the testcase runs.

We need a more representative testcase.
Comment 3 Ron Bodkin CLA 2004-01-21 14:52:58 EST
Here's a revised version that replicates the bug by adding in an interface that 
is woven and that the abstract class implements (which isn't woven). I also 
excluded the aspect from affecting itself so the real error is visible:

Exception in thread "main" java.lang.AbstractMethodError: sample.Derived.getLogg
er()Ljava/util/logging/Logger;
        at sample.Trace.ajc$interMethodDispatch1$sample_Trace$sample_Trace$Trace
d$getLogger(Trace.aj)
        at sample.Trace.ajc$before$sample_Trace$d2(Trace.aj:12)
        at sample.Derived.<init>(Derived.java:6)
        at sample.Derived.main(Derived.java:13)
Comment 4 Ron Bodkin CLA 2004-01-21 14:53:57 EST
Created attachment 7511 [details]
revised test case that exposes the actual bug
Comment 5 Andrew Clement CLA 2004-02-27 10:18:47 EST
Some notes on this bug ... I think there is something that needs fixing, 
possibly with a compiler error message.
 
Explaining Rons program:
an interface 'Iface' is implemented by an abstract class 'Base' which is then 
extended with a class 'Derived'.

All three are compiled into a jar.  We then take 'Base.class' out of that jar.
The resulting jar with just two classes in it is woven against a Trace aspect. 
(The compiler/weaver can still find Base.class because it is accessible on the 
CLASSPATH). The 'Trace' aspect includes an inner Traced interface onto which 
it ITDs a field and a method.  The aspect declares that all the classes apart 
from itself implement 'Trace'.

We then execute the Derived class which blows up trying to find an 
implementation of the method that was introduced to the interface (getLogger).

I *think* that because getLogger is introduced onto an interface, its 
implementation would normally be introduced into top-most implementors.  
During the compilation that involved the aspect, Base.class was on CLASSPATH 
so not a target for weaving.  This means getLogger() was not introduced onto 
it, even though the abstract method getLogger was introduced upon the 
Trace.Traced interface (so at runtime this manifests as an 
AbstractMethodError).  It seems that in the inheritance hierarchy we 
considered Base.class to be the top most implementor of Traced - even though 
it couldn't be woven?

Tracing the compile that involved the 2 class jar and the aspect, I see a 
mungeNewMethod() for the getLogger ITD on the interface Traced and I see a 
mungeNewField() for the ITDd field on Traced.  I don't see (obviously) the 
mungeNewMethod() for the getLogger implementation on Base.class.

I think we should have thrown an error on this compile when we couldn't 
introduce the method implementation into the right place?

However... it is possibly more subtle than that because if I take the 
interface Iface entirely out of the frame, so I simply have an abstract class 
Base and its subclass Derived and I repeat the steps above, it works - in this 
situation the getLogger() implementation is put into Derived ?!?

phew.
Comment 6 Jim Hugunin CLA 2004-03-04 21:38:20 EST
This bug has strong similarities to #52107.  In this case it seems that the 
compiler isn't checking that it has access to the top-most implementors of an 
interface that has a concrete introduction on it.  My guess is that you want 
to modify the same ResolvedTypeMunger.matches method to also do this check for 
the top-most implementors when the target type is an interface.
Comment 7 Adrian Colyer CLA 2004-03-16 04:56:02 EST
updated to milestone 1.2
Comment 8 Adrian Colyer CLA 2004-08-09 15:25:07 EDT
marked as target 1.2.1
Comment 9 Andrew Clement CLA 2004-08-19 10:28:07 EDT
Progress. And it explains my confused comment made a couple of months back.

We have interface 'Iface' and implementor of that 'Base' and subclass of Base
called 'Derived'.  Iface and Derived are accessible for weaving.

An aspect makes a non-abstract ITD onto an interface called 'Traced' and also a
declare parents statement for everything (Iface/Base/Derived) to implement 'Traced'.

The problem arises due to an ordering issue - our approach is to go through each
type and for each one apply all type mungers then all shadow mungers.  I think
this is still right.  The problem here is that we match the munger for the ITD
against the 'Derived' class and correctly discover (at that point in time) that
it is indeed the top most implementor of 'Traced' because 'Base' wasn't touched
by the declare parents statement.  However ... just after we discover it is
'Derived' we then move on to 'IFace' and apply the declare parents statement to
it.  This means IFace implements Traced and therefore Base now implements Traced
even though we didn't actually touch Base.  What happens later on is that we
make this check:

if (onInterface &&                             // The ITD was on an interface 
    !Modifier.isAbstract(signature.getModifiers()) && // It was not abstract
    gen.genType().isTopmostImplementor(onType)) { // we are looking at the
                                                  // top most implementor.
 // Stick the body of the ITD method in.

This fails because isTopmostImplementor now returns false for 'Derived' at the
point we do the weave - in this situation we quietly do nothing and so you don't
get an implementation of the ITD put anywhere!  

I have changed this slightly so that if we are processing a type that is no
longer the top most implementor (when it used to be) then report an error that
we need access to the proper top most implementor for weaving:

type sample.Base must be accessible for weaving interface inter type declaration
from aspect sample.Trace


It is an unusual situation because you are giving some of the top and bottom of
an inheritance hierarchy to AspectJ but not the middle bit.
Comment 10 Andrew Clement CLA 2004-08-19 12:45:08 EDT
Fix checked in - waiting for build.
Comment 11 Andrew Clement CLA 2004-08-20 08:24:29 EDT
Fix available:

BUILD COMPLETE -  build.349
Date of build: 08/20/2004 10:22:41
Time to build: 93 minutes 15 seconds
Last changed: 08/20/2004 08:18:14
Latest good AspectJ jar available at:
download.eclipse.org/technology/ajdt/dev/aspectj-DEVELOPMENT.jar
Comment 12 Adrian Colyer CLA 2004-10-21 04:32:46 EDT
Fix released as part of AspectJ 1.2.1