AspectJ showing circular advice precedence error where it shouldn't

[ttho]

AspectJ complains about my aspect and I don't understand why.

The original aspect is much more complex, I already boiled it down to something rather simple:

package de.tt;

import java.util.ArrayList;

import org.aspectj.lang.annotation.After;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Before;

@Aspect
public class FaultyAspect {

    @Before("call(* *(..)) && target(java.util.Collection)")
    public void before1() {
    }

    @After("call(* *(..)) && target(java.util.Collection)")
    public void after1() {
    }

    @Before("call(* *(..)) && !target(java.util.Collection)")
    public void before2() {
    }

    @After("call(* *(..)) && !target(java.util.Collection)")
    public void after2() {
    }

    public static void main(String[] args) {
        new ArrayList().iterator().hasNext();
    }
}

The error showing up is (multiple times):

circular advice precedence: can't determine precedence between two or more pieces of advice that apply to the same join point: method-call(boolean java.util.Iterator.hasNext())

I do not understand that error. For me the aspect should be ok. Strangely, the problem goes away when the after1 and after2 advice is removed. The same is true for removing before1 and before2.

Tested with 1.9.19 and 1.9.20-SNAPSHOT (a8a2f82fd)

[kriegaex]

I am too busy to read your inquiry in detail right now, but the basic problem with the current way of determining precedence is described in https://github.com/eclipse/org.aspectj/issues/25. I also have a simple and elegant way to avoid it, introducing new precedence rules which can no longer lead to circularity.

The thing is, it would be a breaking change. I would love to do it, but back in 2020 when I suggested it, I was not an active committer yet, and project lead @aclement neither was convinced that it should be changed nor had any free cycles to look into it. Maybe he and I can discuss this topic again for a future AspectJ release.

For now, just read the other issue I created and try to understand the current precedence rules. I linked to a Stack Overflow answer about this topic and also to the AspectJ manual.

Update: As a goodie, you may use the little intra-aspect precedence simulator tool I also linked off of #25. It visualises your own situation for you, if you configure it correspondingly.

[ttho]

Hi Alex, thanks for your quick reply! I was able to solve my problem with your hints by re-arranging the advices. However, I am still confused why I see this error. The manual states that there might be circularity if pieces of advice "advise the same join point". But this is not the case in my example. I was able to further simplify it to:

@Aspect
public class FaultyAspect {

    @Pointcut("if()")
    public static boolean _true() {
        return true;
    }

    @Pointcut("if()")
    public static boolean _false() {
        return false;
    }

    @Before("call(* *(..)) && _true()")
    public void before1() {
    }

    @After("call(* *(..)) && _true()")
    public void after1() {
    }

    @Before("call(* *(..)) && _false()")
    public void before2() {
    }

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

If before2() is removed, the error vanishes. However, advice before2 clearly does not apply to the same join points.

[kriegaex]

call(* *(..)) does apply to the same pointcuts in all your advices. The if() pointcuts do not change that fact. They only serve to dynamically decide during runtime whether an advice should be executed or not. An if() pointcut could yield different results when called several times, i.e. the advice would be executed or not. But it still matches the same joinpoints.

@Pointcut("if()")
public static boolean throwDice() {
  return new Random().nextBoolean();
}

[ttho]

Oh yes, poorly simplified. I should have done it like this:

@Aspect
public class FaultyAspect {

    @Before("call(* *(..)) && target(java.util.List)")
    public void before1() {
    }

    @After("call(* *(..)) && target(java.util.List)")
    public void after1() {
    }

    @Before("call(* *(..)) && !target(java.util.List)")
    public void before2() {
    }

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

If I replace 'java.util.List' by 'java.util.ArrayList', the error vanishes. So it comes down to that some pointcuts can only by evaluated at runtime, in this case whether the target implements an interface. For classes (inheritance), this can be determined at weave time.

[kriegaex]

I think we have clarified everything here. AspectJ behaves as it is supposed to according to the current intra-aspect precedence rules, even though I would also prefer the ones I suggested, in which circularity cannot even occur. But this is not how AspectJ implements them at present.