Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [aspectj-dev] around advice & exceptions

Hi Rafal -

This is an important and difficult area :)  Thanks for noticing
and following up on the bug workaround.

First, a side note: your proxy pattern invokes the method directly,
rather than using proceed(..). As a result, any less-precedent advice
will not run.  It's better to close over the proceed(..) invocation
and pass that to the method, perhaps using a nested class that
permits the callee to modify the context used to proceed.  Doing
that will cause other advice to run properly and also works for
join points besides method-execution.

For Jan Hannemann's AspectJ implementation of the proxy pattern, 
see the link from the samples page linked off the AspectJ site 
documentation page.

Second (as background) enforcing exceptions in Java is the 
responsibility of the compiler, so in theory anyone who wants 
to futz with bytecode can throw checked exceptions that 
weren't declared.  (Ron Bodkin put something to do this in 
open-source somewhere.)  But AspectJ is careful to require that any 
checked exceptions thrown by the advice must be declared by the advice
and the underlying join point, so users don't lose any of Java's
exception handling guarantees.  In theory, your proposal doesn't 
expand what the join point can throw, so any advice or code that gets 
control in an exception handler should be expecting the exception.
I'd want to verify that.  (I'm less happy about using "throws 
UnwrappableException" to mean "this advice unwraps any 
UnwrappableException it can" but I think that's a separate issue.)

Also, doing this exception macro thingy at each join point would entail 
every implementation of AspectJ doing the same thing, so we might
be committing them to a particular implementation of around advice.

I'm not sure yet why you need to catch and rethrow the exceptions.
You seem to be using reflection (and having to unwrap things like
InvocationTargetException) rather than just changing the context
using proceed.  E.g.,

    Object around(Object original)  : proxied(original) {
        Object result = originalToProxy.get(original);
        return proceed(null == result ? original : result);
    }

(Note that you can get a lot of benefit from using the supertype of
some context object, because it enables you to pick out join points
whose context objects are different types but share the same supertype.
However, you will cause a ClassCastException if your replacement object
is not the right type for this join point.)

> A note about <<< workaround for AJ bug >>>

If your NPE is reproducible in the latest version of AspectJ,
please submit it as a bug.

Thanks -
Wes


> ------------Original Message------------
> From: Rafal Krzewski <Rafal.Krzewski@xxxxxxxxx>
> To: aspectj-dev@xxxxxxxxxxx
> Date: Thu, Mar-24-2005 0:11 AM
> Subject: [aspectj-dev] around advice & exceptions
>
> Hello,
> 
> I've started a toy project for myself to explore AOP a bit. While 
> working on it I came across something that I believe is a limitation of 
> 
> AspectJ language.
> 
> I am trying to write a generic aspect for proxy pattern:
> 
> import org.aspectj.lang.JoinPoint;
> import org.aspectj.lang.reflect.MethodSignature;
> 
> public abstract aspect TransparentProxy perthis(association(Object)) {
> 
>    public abstract pointcut association(Object  o);
> 
>    public abstract pointcut proxyOperations(Object  o);
> 
>    private Object delegate;
> 
>    public void setDelegate(Object delegateArg)  {
>      delegate  =  delegateArg;
>    }
> 
>    Object around(Object  o) : proxyOperations(o)
>    {
>      if (delegate == null)
>        return proceed(o);
>      else
>        return proxyProceed(o, thisJoinPoint);
>    }
> 
>    private Object proxyProceed(Object target, JoinPoint tjp) {
>      MethodSignature sig = ((MethodSignature) tjp.getSignature());
>      sig.getDeclaringType(); // <<< workaorund for AJ bug >>>
>      try  {
>        return sig.getMethod().invoke(target, tjp.getArgs());
>      } catch (IllegalAccessException e) {
>        throw new RuntimeException("reflection problem", e);
>      } catch (InvocationTargetException e) {
>        throw new RuntimeException("business exception", 
> e.getTargetException()); // <<< limitation here >>>
>      }
>    }
> }
> 
> The problem that I am seeing is that a generic around proxyOperations 
> clause is not able to declare what checked exceptions will be thrown 
> inside it, because it is expected to capture joint points with 
> signatures varying in this regard. This makes it impossible to properly 
> 
> unwrap and rethrow the exception reported in InvocationTargetException 
> by the method invoked reflectively in the aspect above.
> 
> I've read about exception (re)introduction pattern in AspectJ in Action 
> 
> book, and I have the impression that a generic solution to this problem 
> 
> is not possible as of today.
> 
> I think I see a possibility of enhancing AspectJ to handle this.
> 
> Assumption: The weaver can statically determine the types of exceptions 
> 
> that can be thrown in the context where the advice is being woven in.
> 
> 1) create new exception type org.aspectj.lang.UnwrappableException that 
> 
> would carry an inner Throwable object.
> 
> 2) when an advice declares it can throw the above exception, weave it 
> in 
> within a try/catch block, and perform the following logic in the catch 
> block per each exception type that can be thrown at the join point 
> being 
> processed: check if the caught WrappableException carries the exception 
> 
> type, if yes throw it, if not continue. If none of the checks results 
> in 
> a throw, the WrappableException contained invalid exception - this also 
> 
> needs to be signalled, with an RuntimeException or Error.
> 
> Excuse me if I am not making sense: I have no idea about weaver 
> internals.
> 
> A note about <<< workaround for AJ bug >>> line in my example above:
> I am getting NPE when calling sig.getMethod() directly. A quick 
> investigation revealed that declaringType field was null. A naive fix 
> would be:
> 
> public Method getMethod() {
>    if (method == null) {
>      if(getDeclaringType() != null) {
>        try {
>          method = 
> declaringType.getDeclaredMethod(getName(),getParameterTypes());
>        } catch (NoSuchMethodException nsmEx) {
>          ; // nothing we can do, user will see null return
>        }
>      }
>    }
>    return method;
> }
> 
> but 1) there are more methods like that getField(), getConstructor() 
> and 
> so on, and if the fix is valid it should be applied to all of them, 2) 
> instead of doint that it may be more important to investigate why the 
> constructor was called with null declaringType arg in the first place.
> 
> Rafal
> _______________________________________________
> aspectj-dev mailing list
> aspectj-dev@xxxxxxxxxxx
> http://dev.eclipse.org/mailman/listinfo/aspectj-dev
> 



Back to the top