Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with abstract aspects and cflow(within()) #238

Open
ttho opened this issue Apr 24, 2023 · 4 comments
Open

Problem with abstract aspects and cflow(within()) #238

ttho opened this issue Apr 24, 2023 · 4 comments
Assignees
Labels
question Further information is requested

Comments

@ttho
Copy link

ttho commented Apr 24, 2023

Given the following abstract and concrete aspect:

package de.tt;

import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Before;
import org.aspectj.lang.annotation.Pointcut;

@Aspect
public abstract class AbstractAspect {
    
    @Pointcut
    abstract void myPointcut();

    @Before("myPointcut()")
    public void before(JoinPoint jp) {
        System.out.println("before " + jp);
    }
}
package de.tt;

import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Pointcut;

@Aspect
public class ConcreteAspect extends AbstractAspect {

    @Pointcut("cflow(within(de.tt.ConcreteAspect)) || cflow(within(de.tt.AbstractAspect))") 
    public void self() {
    }
    
    @Pointcut("!self() && call(* *(..))")
    public void myPointcut() {
    }

}

I get the following error when load-time weaving

package de.tt;

public class Main {

    public static void main(String[] args) {
        System.out.println("hello");
    }

}

Exception in thread "main" java.lang.ExceptionInInitializerError at de.tt.Main.main(Main.java:6) Caused by: java.lang.NullPointerException at de.tt.AbstractAspect.<clinit>(AbstractAspect.java:1) ... 1 more

Why is that? Interestingly, the error goes away when

  • replacing cflow(within(de.tt.AbstractAspect)) with within(de.tt.AbstractAspect), or
  • defining the concrete aspect through XML:
<aspectj>
   <aspects>
       <concrete-aspect name="de.tt.ConcreteAspect2" extends="de.tt.AbstractAspect">
           <pointcut name="myPointcut" expression="!cflow(within(de.tt.ConcreteAspect)) &amp;&amp; !cflow(within(de.tt.AbstractAspect)) &amp;&amp; call(* *(..))" />
       </concrete-aspect>
   </aspects>
</aspectj>

Tested with a8a2f82.

Background: I am writing a very general aspect for access tracking and want to exclude any calls from my aspects. Is there a better way to achieve that?

@kriegaex
Copy link
Contributor

Sorry for replying so late. Thanks for the problem report. I understand why you might want to use that pointcut. If your tracing aspect is the only one active and there is nothing like nested aspect execution, you could use !cflow(adviceexecution()) && call(* *(..)) as a workaround. But if you use other aspects around (not before or after) the same code you want to trace, the corresponding calls would be excluded.

The problem you are describing originates in the fact that the base aspect constructor wants to use a control flow counter from the sub aspect. That counter is static and should be initialised when the sub aspect is going through static initialisation, i.e. during class-loading. I would have expected that that would have happened already when the base aspect constructor is executing. Why this is not the case, is yet to be investigated. A workaround for now would be not to extend a base aspect but to put everything into a single aspect.

@kriegaex
Copy link
Contributor

kriegaex commented Jun 14, 2023

A better solution would be, adapted from an example in the AspectJ Programming Guide:

pointcut myAdvice():   adviceexecution() && within(AbstractAspect+);
pointcut self():       cflow(myAdvice());
pointcut myPointcut(): !self() && call(* *(..));

The addition of && within(AbstractAspect+) makes it more precise. Sorry for the native AspectJ syntax. I guess you know how to convert that to annotation syntax.

You can, of course, also merge the first two pointcuts into one:

pointcut self():       cflow(adviceexecution() && within(AbstractAspect+));
pointcut myPointcut(): !self() && call(* *(..));

Unrelated to this issue, but an idea for you: If you also want to trace constructor calls, you could do this:

pointcut myPointcut(): !self() && (call(* *(..)) || call(*.new(..)));

The trace would still miss staticinitialization, preinitialization and initialization pointcuts, if these would be of any interest, but your own pointcut is just fine for method calls.

@kriegaex kriegaex self-assigned this Jun 14, 2023
@kriegaex
Copy link
Contributor

I think, I am not going to do anything to cover the corner case you just uncovered, because there is a good, canonical workaround. Generated aspect byte code would get more complex and potentially slower, too. The problem with within(AbstractAspect+) is, that it matches many joinpoints, among them static class initialisation, instance pre-initialisation and initialisation. Combining it with adviceexecution() is easy and expresses more clearly what you want in this case.

@aclement, do you think this ought to be fixed or improved?

@kriegaex kriegaex added the question Further information is requested label Jun 14, 2023
@ttho
Copy link
Author

ttho commented Aug 8, 2023

@kriegaex, sorry for late feedback. With your explanations I was able to fix my problem, thank you very much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants