Summary: | Inline constructor code initialising final instance fields | ||||||
---|---|---|---|---|---|---|---|
Product: | [Tools] AspectJ | Reporter: | Alexander Kriegisch <Alexander> | ||||
Component: | Compiler | Assignee: | aspectj inbox <aspectj-inbox> | ||||
Status: | NEW --- | QA Contact: | |||||
Severity: | normal | ||||||
Priority: | P3 | CC: | davidgeorg_reichelt, jinya.namiki | ||||
Version: | 1.9.5 | ||||||
Target Milestone: | --- | ||||||
Hardware: | PC | ||||||
OS: | Windows 10 | ||||||
Whiteboard: | |||||||
Attachments: |
|
Description
Alexander Kriegisch
2020-05-29 06:27:21 EDT
I forgot the actual aspect code: package de.scrum_master.aspect; import de.scrum_master.app.Base; import de.scrum_master.app.Sub; public aspect ConstructorAspect { // before() : preinitialization(Base+.new(..)) { // System.out.println(" " + thisJoinPoint); // } // // before() : initialization(Base+.new(..)) { // System.out.println(" " + thisJoinPoint); // } void around() : execution(Base+.new(..)) && target(Sub) { // System.out.println(" " + thisJoinPoint); return; } } The commented out stuff was just for my better understanding when tracing what was happening there. Andy, someone else hit the same problem in an different context. I think it needs fixing. See https://stackoverflow.com/q/64611859/1082681 and my answer there. I am unsure whether inlining the methods will work. I tried to always inline constructors, regardless of whether canInline is false: https://github.com/DaGeRe/org.aspectj/commit/3d184d1ab07638f0d69e0f9ce8be49495089339c While the inline-weaving seems to be finished (the System.out at the end of BcelShadow.weaveAroundInline is printed), the error still appears. As far as I understand it, the problem seems to be that an extracted method is created, and that Java 11 does not allow modifications of final variables by this extracted method. Is this right? Therefore, weaving into constructors of classes with final fields is theoretically only possible if proceed is only called once. To make it work, the original shadow would need to be re-inlined. Is this correct? I did not analyse the AspectJ source code or your modification of same. But are you sure that your change actually led to inlining the constructor advice into the original constructor. That your log line is printed after instrumentation is done does not mean inlining was successful. I am not even sure if AspectJ can inline an constructor advice at present. Better check the resulting byte code, e.g. via javap. Anyway, for now I posted a workaround in my answer to your StackOverflow question. I don't like it much, hence the name workaround. But it works in the context of your timing requirement. Created attachment 284635 [details]
Woven version of the FinalFieldConstructorExample
Since I am not fully familiar with the internals of AspectJ and bytecode, my assumption was from the documentation, which states: "AroundInline still extracts the instructions of the original shadow into an extracted method. This allows inlining of even that advice that doesn't call proceed or calls proceed more than once. [...] If only one call to proceed is made, we can re-inline the original shadow. We are not doing that presently." Thanks to your hint, I checked what ajc is doing when I execute build-time weaving on my example (both with and without my changes to the inlining-part). In both cases, i.e. also using the standard 1.9.7 from github without modifications, I do not see a matching method declaration of init$_aroundBody2 in the bytecode of FinalFieldConstructorExample (see attachement, it can be created using https://github.com/DaGeRe/aspect-final-example/blob/main/woven-project/analyzeBytecode.sh). Only a static init$_aroundBody2 (without access to parameters) is present, but this seems to be fine to me, since it only concerns the static initializer. Therefore, I do not understand why the error occurs, and do therefore not see how to fix this. Well, the FinalFieldConstructorExample constructor according to Javap is: public de.scrum_master.app.FinalFieldConstructorExample(); Code: 0: aload_0 1: invokespecial #10 // Method java/lang/Object."<init>":()V 4: getstatic #82 // Field ajc$tjp_1:Lorg/aspectj/lang/JoinPoint$StaticPart; 7: aload_0 8: aload_0 9: invokestatic #85 // Method org/aspectj/runtime/reflect/Factory.makeJP:(Lorg/aspectj/lang/JoinPoint$StaticPart;Ljava/lang/Object;Ljava/lang/Object;)Lorg/aspectj/lang/JoinPoint; 12: astore 5 14: invokestatic #75 // Method de/scrum_master/aspect/MyAspect.aspectOf:()Lde/scrum_master/aspect/MyAspect; 17: iconst_2 18: anewarray #3 // class java/lang/Object 21: astore 6 23: aload 6 25: iconst_0 26: aload_0 27: aastore 28: aload 6 30: iconst_1 31: aload 5 33: aastore 34: new #90 // class de/scrum_master/app/FinalFieldConstructorExample$AjcClosure3 37: dup 38: aload 6 40: invokespecial #91 // Method de/scrum_master/app/FinalFieldConstructorExample$AjcClosure3."<init>":([Ljava/lang/Object;)V 43: dup 44: astore 7 46: ldc #92 // int 69648 48: invokevirtual #69 // Method org/aspectj/runtime/internal/AroundClosure.linkClosureAndJoinPoint:(I)Lorg/aspectj/lang/ProceedingJoinPoint; 51: invokevirtual #79 // Method de/scrum_master/aspect/MyAspect.aroundStuff:(Lorg/aspectj/lang/ProceedingJoinPoint;)Ljava/lang/Object; 54: pop 55: return Please note that at the end it created a FinalFieldConstructorExample$AjcClosure3 object. Now what does that class look like? Compiled from "FinalFieldConstructorExample.java" public class de.scrum_master.app.FinalFieldConstructorExample$AjcClosure3 extends org.aspectj.runtime.internal.AroundClosure { public de.scrum_master.app.FinalFieldConstructorExample$AjcClosure3(java.lang.Object[]); Code: 0: aload_0 1: aload_1 2: invokespecial #10 // Method org/aspectj/runtime/internal/AroundClosure."<init>":([Ljava/lang/Object;)V 5: return public java.lang.Object run(java.lang.Object[]); Code: 0: aload_0 1: getfield #14 // Field org/aspectj/runtime/internal/AroundClosure.state:[Ljava/lang/Object; 4: astore_2 5: aload_2 6: iconst_0 7: aaload 8: checkcast #16 // class de/scrum_master/app/FinalFieldConstructorExample 11: aload_2 12: iconst_1 13: aaload 14: checkcast #18 // class org/aspectj/lang/JoinPoint 17: invokestatic #22 // Method de/scrum_master/app/FinalFieldConstructorExample.init$_aroundBody2:(Lde/scrum_master/app/FinalFieldConstructorExample;Lorg/aspectj/lang/JoinPoint;)V 20: aconst_null 21: areturn } Here you see that the 'run' method calls method FinalFieldConstructorExample.init$_aroundBody2. That one looks as follows: static final void init$_aroundBody2(de.scrum_master.app.FinalFieldConstructorExample, org.aspectj.lang.JoinPoint); Code: 0: aload_0 1: iconst_5 2: istore_2 3: getstatic #44 // Field ajc$tjp_0:Lorg/aspectj/lang/JoinPoint$StaticPart; 6: aload_0 7: aconst_null 8: iload_2 9: invokestatic #50 // Method org/aspectj/runtime/internal/Conversions.intObject:(I)Ljava/lang/Object; 12: invokestatic #56 // Method org/aspectj/runtime/reflect/Factory.makeJP:(Lorg/aspectj/lang/JoinPoint$StaticPart;Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)Lorg/aspectj/lang/JoinPoint; 15: astore 4 17: invokestatic #75 // Method de/scrum_master/aspect/MyAspect.aspectOf:()Lde/scrum_master/aspect/MyAspect; 20: iconst_3 21: anewarray #3 // class java/lang/Object 24: astore 6 26: aload 6 28: iconst_0 29: aload_0 30: aastore 31: aload 6 33: iconst_1 34: iload_2 35: invokestatic #50 // Method org/aspectj/runtime/internal/Conversions.intObject:(I)Ljava/lang/Object; 38: aastore 39: aload 6 41: iconst_2 42: aload 4 44: aastore 45: new #60 // class de/scrum_master/app/FinalFieldConstructorExample$AjcClosure1 48: dup 49: aload 6 51: invokespecial #63 // Method de/scrum_master/app/FinalFieldConstructorExample$AjcClosure1."<init>":([Ljava/lang/Object;)V 54: dup 55: astore 8 57: sipush 4096 60: invokevirtual #69 // Method org/aspectj/runtime/internal/AroundClosure.linkClosureAndJoinPoint:(I)Lorg/aspectj/lang/ProceedingJoinPoint; 63: invokevirtual #79 // Method de/scrum_master/aspect/MyAspect.aroundStuff:(Lorg/aspectj/lang/ProceedingJoinPoint;)Ljava/lang/Object; 66: checkcast #13 // class java/lang/Integer 69: putfield #18 // Field parameters:Ljava/lang/Integer; 72: return And here we are! Look what happens before 'return': 69: putfield #18 // Field parameters:Ljava/lang/Integer; Write access to the final field, which is forbidden outside the constructor where it originally happens in the unwoven class. So your AspectJ patch by no means inlined what actually needed to be inlined, i.e. the whole around-aspect code into the original constructor. You still have a helper class + static helper method. So if Andy would implement what I requested in this ticket, your problem would be solved. But whatever you patched in AspectJ does not do the job. I'm using AspectJ 1.9.7. Is this problem still remaining? Yes. |