Community
Participate
Working Groups
Created attachment 144346 [details] Eclipse projects showing the bug Build ID: 20090619-0625 Steps To Reproduce: 1. Load the two projects attached to this bug inside eclipse 2. Place the TestLTWDeclareParentsStuff on the inpath of TestLTWDeclareParents 3. Run the junit test, you'll see that MarkerInterface and its doSomething() method are weaven (correctly) in TopMost, and not in Child or GrandChild. 4. Remove TestLTWDeclareParentsStuff from the inpath 5. Make sure you clean the project 6. Run the junit test with LTW (it contains a main so that it can be run from there) 7. See that this time the interface and relative ITD method are woven into GrandChild, simply because that one is loaded before Child or TopMost. ----- If the project is not cleaned, or if the two project are placed in a single one, apparently it will work correctly, but that happens only because LTW reads an already weaved TopMost, and properly avoid placing the MarkerInterface on GrandChild and Child. More information: While in CTW, the weaver is able to span all the classes, and correctly identify parents and apply interfaces and relative methods to topmost possible classes. While in LTW, the weaver avoids loading non already loaded classes, which is fine, except that in this case we could end up having more than one class in the hierarchy implementing the same interfaces and same methods. This is not a problem per se, except when those classes are reflected in search for annotations and multiple methods and annotations are found in the hierarchy. JPA for example is very sensitive to these situations.
After a long debugging session, I can see that when AspectJ weaves a loaded class, inside BcelWeaver.weave line 1217, it explicitly calls for a list of types to consider while weaving declare parents or declare @type. I suppose this list should contain all possible affected types. In fact, the comment on method weaveParentsFor say "when supplying partial hierarchies, this algorithm may break down. If you have a hierarchy A>B>C and only give A and C to the weaver, it may choose to weave them in either order - but you'll probably have other problems if you are supplying partial hierarchies like that !". However, the WeavingAdaptor.WeavingClassFileProvider used in LTW is giving only the current class, so it is supplying a partial hierarchy. The weaver then does not consider parent classes, simply weaves on the current one, causing subsequent loading of parent class to re-weave the same interface/methods/annotations that were already placed on subclasses. During CTW instead, the class WeavingAdaptor is used, which return a quite big list of UnwovenClassFiles.
I'm experimenting a simple patch, that populates the WeavingAdaptor.WeavingClassFileProvider.unwovenClasses list also with UnwovenClassFile's for parents, having the bcelWorld load them. It seems to solve the current problem, but there could be a number of side effects I can't see. It seems to weave parent classes each time, which potentially slows down things quite a bit for complex hierarchies. Defining them using IWeaveRequestor.acceptResult would require a guard to avoid defining them twice in the classloader, so I'm currently discarding them, but this time could be saved if the woven parent classes were defined. Also, I'm not yet adding interfaces.
those test projects use something called 'hamcrest' - i dont have
the infrastructure that may help here is 'complete binary types' - which is activated via: <weaver options="-Xset:completeBinaryTypes=true"/> this produces different output but as the test doesn't actually check for existence of the methods on the right type, the test doesn't fail. It looks like the methods from the ITD just don't get put anywhere. This is why completeBinaryTypes is currently turned off by default, since it isn't fully tested...
Hi Andy, hamcrest is an expression system used by junit 4. I will rephrase the test to use basic junit. Also, I'll attach my dummy patch. It seems to solve this specific problem, but I don't manage to run tests on AspectJ so I don't know if it breaks something else.
dont worry about reworking the test, I already did that.
Created attachment 147656 [details] A patch making the test case runc orrectly
The attached patch make the following : when a class is going to be weaved, load also its superclasses and fill the list also with superclasses. This way the weaver has visibility on all the hierarchy, and can apply the ITDs in the correct way (superclass first) as it does during compile time weaving. The impact is minimal, because superclasses are a small set and will be loaded anyway soon during a program execution. I haven't been able to test this patch completely, but I had both the test project attached here and my original program run correctly once this patch is applied.
right, I am properly looking at this. I've converted the test program into a proper pair of binary weaving and ltw tests. I renamed the types top/middle/bottom. On binary weaving I get: Type 'Mark$IMarker' (Mark.java) has intertyped method from 'Mark' (Mark.java:'java.lang.String Mark$IMarker.markMethod()') Extending interface set for type 'Top' (Top.java) to include 'Mark$IMarker' (Mark.java) Type 'Top' (Top.java) has intertyped method from 'Mark' (Mark.java:'java.lang.String Mark$IMarker.markMethod()') I then ran the same thing with LTW and got: [WeavingURLClassLoader] weaveinfo Extending interface set for type 'Bottom' (Bottom.java) to include 'Mark$IMarker' (Mark.java) [WeavingURLClassLoader] weaveinfo Type 'Bottom' (Bottom.java) has intertyped method from 'Mark' (Mark.java:'java.lang.String Mark$IMarker.markMethod()') [WeavingURLClassLoader] weaveinfo Type 'Mark$IMarker' (Mark.java) has intertyped method from 'Mark' (Mark.java:'java.lang.String Mark$IMarker.markMethod()') [WeavingURLClassLoader] weaveinfo Extending interface set for type 'Middle' (Middle.java) to include 'Mark$IMarker' (Mark.java) [WeavingURLClassLoader] weaveinfo Type 'Middle' (Middle.java) has intertyped method from 'Mark' (Mark.java:'java.lang.String Mark$IMarker.markMethod()') [WeavingURLClassLoader] weaveinfo Extending interface set for type 'Top' (Top.java) to include 'Mark$IMarker' (Mark.java) [WeavingURLClassLoader] weaveinfo Type 'Top' (Top.java) has intertyped method from 'Mark' (Mark.java:'java.lang.String Mark$IMarker.markMethod()') All three (Bottom/Middle/Top) get the marker interface and the intertyped method. I then turned on the weaver option -Xset:completeBinaryTypes=true as per my previous comment and reran ltw: [WeavingURLClassLoader] weaveinfo Extending interface set for type 'Top' (Top.java) to include 'Mark$IMarker' (Mark.java) [WeavingURLClassLoader] weaveinfo Type 'Top' (Top.java) has intertyped method from 'Mark' (Mark.java:'java.lang.String Mark$IMarker.markMethod()') [WeavingURLClassLoader] weaveinfo Type 'Mark$IMarker' (Mark.java) has intertyped method from 'Mark' (Mark.java:'java.lang.String Mark$IMarker.markMethod()') Back to the 3 messages we want to see. Simone - did you try -Xset:completeBinaryTypes=true?
testcode committed. see ajc166.xml and Ajc166Tests.java
Hi Andy, in fact I had that patch on my hd far before you suggested the completeBinaryTypes . I have not yet tested it in my complete scenario simply for lack of time, but will surely give it a try as soon as I manage to have the time.
unsetting the target field which is currently set for something already released
