Bug 348071 - !adviceexecution() doesn't seem to prevent after throwing advice from applying
Summary: !adviceexecution() doesn't seem to prevent after throwing advice from applying
Status: NEW
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: 1.6.11   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-02 10:44 EDT by Gary T. Leavens CLA
Modified: 2011-06-02 14:30 EDT (History)
1 user (show)

See Also:


Attachments
The aspect described in the report (550 bytes, application/octet-stream)
2011-06-02 10:44 EDT, Gary T. Leavens CLA
no flags Details
The main program described in the report (487 bytes, application/octet-stream)
2011-06-02 10:47 EDT, Gary T. Leavens CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gary T. Leavens CLA 2011-06-02 10:44:23 EDT
Created attachment 197231 [details]
The aspect described in the report

If code throws a runtime exception from within after returning advice, then after throwing advice that has a pointcut description that is conjoined with !adviceexecution() catches that exception, even though the exception was not thrown within the orginal method call or execution described in the joinpoint.

This might actually be a bug in the documentation, since it's not clearly described in the programmer's guide what adviceexecution() means.

Here's some code that shows the problem:

// file Refactoring3AspectRewritten.aj
privileged aspect Refactoring3AspectRewritten {
    pointcut RFPointcut() : execution(void setx(..)) && within(Refactoring3)
    	     		   && !adviceexecution();
    before() : RFPointcut() {
            System.err.println("Before proceed");
    }
    after() returning : RFPointcut() {
            System.err.println("After proceed.. but before throw");
	    throw new RuntimeException("oops");
    }
    after() throwing(RuntimeException e) : RFPointcut() {
            System.err.println("In exception handler for " + e);
    }
}

// file Refactoring3.java
public class Refactoring3 {
    int x = 0;
    void setx(int v) {
        x = v;
    }
    public static void main(String[] argv) {
        Refactoring3 obj = new Refactoring3();
        try {
            obj.setx(7);
        } catch(RuntimeException e) {
            System.err.println("got exception in main");
            e.printStackTrace();
        }
    }
}

Here's what happens when I run this:

$ rm *.class; ajc Refactoring3AspectRewritten.aj Refactoring3.java; java Refactoring3; java -version
Before proceed
After proceed.. but before throw
In exception handler for java.lang.RuntimeException: oops
got exception in main
java.lang.RuntimeException: oops
        at Refactoring3AspectRewritten.ajc$afterReturning$Refactoring3AspectRewr
itten$2$eff45887(Refactoring3AspectRewritten.aj:9)
        at Refactoring3.setx(Refactoring3.java:5)
        at Refactoring3.main(Refactoring3.java:9)
java version "1.6.0_24"
Java(TM) SE Runtime Environment (build 1.6.0_24-b07)
Java HotSpot(TM) Client VM (build 19.1-b02, mixed mode, sharing)

(I also get the same output, except for the Java verison, in Java 1.4.2.)

You can see from the stack printout that the exception was thrown from inside the advice. So I would think that !adviceexecution() should stop the after throwing advice from running, but in fact the after throwing advice does run. The AspectJ programming guide is not very clear about what should happen, so it may be that the compiler is right and the documentation needs to be made clear. But I think that the idea of adviceexecution() as a point cut should be simple and understandable, and that this execution does not satisfy the obvious simple idea.
Comment 1 Gary T. Leavens CLA 2011-06-02 10:47:10 EDT
Created attachment 197233 [details]
The main program described in the report
Comment 2 Andrew Clement CLA 2011-06-02 14:30:23 EDT
Couple of things I can talk about here:

- after throwing
After throwing advice will trigger if the selected joinpoint exits via exception.  Although your advice throws the exception, the joinpoint you are matching on *also* exit by exception as the RuntimeException passes into setx but doesn't catch it, and so it does exit via exception.  Basically you aren't choosing whether to catch it based on who throws it, you are choosing to catch it based on whether a particular method exits via that exception.

- adviceexecution
I agree this can be a tricky one to understand initially.  But basically it is very similar to execution().  Where execution() is for regular methods/constructors.  adviceexecution() does exactly the same thing for advice.  Although it *could*, adviceexecution() doesn't take any parameters (advice is anonymous, but you could imagine it selecting before/after/around or specifying the advice parameters).  

A joinpoint is never going to match execution() and adviceexecution() at the same time (it will either be a method running and match execution() or advice running and match adviceexecution()).  This is why your initial pointcut augmented with !adviceexecution() makes no difference:

execution(void setx(..)) && within(Refactoring3) && !adviceexecution();

The first bit already says "i am only interested in the execution of 'void setx(..))'" - that can never match advice executing, so adding !adviceexecution() will not change what it matches at all.

- ordering
You could, in this particular case, change the order of your after() advice (have the throwing before the returning).  That would mean the throwing advice doesn't trigger as the exception is thrown after.  But if this is just a small representative case of the bigger situation you are trying to solve, that is probably not the solution you want.

what can you do? Simplest is inspecting the exception stack probably (and ignoring anything whose first stack trace element is setx), although performance won't be great.