Strange behaviour with AOP

[Anthony MULLER]

Hello,

When I use AOP feature, sometimes I have a strange behaviour and I don't know why...

When I call mi.proceed(); into invoke() method of the interceptor, the interceptor's invoke() method is called again! But the intercepted method is called once has expected...

Why the interceptor is called twice sometimes?

Regards,
Anthony MÜLLER

[Anthony MULLER]

Hmmm... With debugger, when I looked to MethodInvocation instance given to invoke() method, I saw something quite strange.

in the "proxy" field of "mi" (MethodInvocation  instance), there are many "CGLIB$CALLBACK_" fields... Ok...

BUT : I found two similars CGLIB$CALLBACK_0 and CGLIB$CALLBACK_1

The target methods are respectively :

public volatile com.xxx.MyObject com.xxx.MyObjectServiceImpl.createXXX(com.xxx.Session)
public com.xxx.MyObject com.xxx.MyObjectServiceImpl.createXXX(com.xxx.Session)

So, the same method is present twice, with just "volatile" as difference...

Why???

Another method into the same class doesn't have this behaviour (interceptor calling interceptor again calling the real method...)

Regards,
Anthony MÜLLER

2008/9/12 Anthony MULLER <anthony...@gmail.com>

[tzwoenn]

I had the same problem, but it was caused by myself. Actually I had
bound the same interceptor during for class level und method level
interception, like

bindInterceptor(any(), annotatedWith(Interceptors.class),
javaxInterceptor);
bindInterceptor(annotatedWith(Interceptors.class), any(),
javaxInterceptor);

If both - @Interceptors at class and method level - are found, the
interceptor is called twice. This could be solved by

bindInterceptor(any(), annotatedWith(Interceptors.class),
javaxInterceptor);
bindInterceptor(annotatedWith(Interceptors.class),
not(annotatedWith(Interceptors.class)), javaxInterceptor);

Maybe you made a similar mistake while configuring the interceptor?

On Sep 12, 2:06 pm, "Anthony MULLER" <anthony.mul...@gmail.com> wrote:
> Hmmm... With debugger, when I looked to MethodInvocation instance given to
> invoke() method, I saw something quite strange.
>
> in the "proxy" field of "mi" (MethodInvocation  instance), there are many
> "CGLIB$CALLBACK_" fields... Ok...
>
> BUT : I found two similars CGLIB$CALLBACK_0 and CGLIB$CALLBACK_1
>
> The target methods are respectively :
>
> public volatile com.xxx.MyObject
> com.xxx.MyObjectServiceImpl.createXXX(com.xxx.Session)
> public com.xxx.MyObject
> com.xxx.MyObjectServiceImpl.createXXX(com.xxx.Session)
>
> So, the same method is present twice, with just "volatile" as difference...
>
> Why???
>
> Another method into the same class doesn't have this behaviour (interceptor
> calling interceptor again calling the real method...)
>
> Regards,
> Anthony MÜLLER
>

> 2008/9/12 Anthony MULLER <anthony.mul...@gmail.com>

[Anthony MULLER]

Hello,

Thanks for this explanation, it will be helpful in future!

My mistake was quite different I think.. I wrote my own method matcher and I don't know all the rules of reflection in Java. I just add a trick to "forget" method this "volatile" modifier.

I'd like somebody from Guice Team looked at this and told me if it is ok or not?

Cheers,
Anthony

    /**
     * Search for the method signature into all the interface annotated with a
     * given annotation.
     */
    public boolean matches(Method method) {
        // Hook to avoid twice interceptions of the same method
        if (Modifier.isVolatile(method.getModifiers())) {
            return false;
        }

        boolean matches = false;
        Class<?> type = method.getDeclaringClass();
        Class<?>[] iTypes = ReflectUtils.getInterfaces(type, annClass);
        for (Class<?> iType : iTypes) {
            Method iMethod = null;

            try {
                iMethod = iType.getDeclaredMethod(method.getName(), method
                        .getParameterTypes());
            } catch (SecurityException e) {
                // Should never happen
            } catch (NoSuchMethodException e) {
                // Should never happen
            }
            if (iMethod != null) {
                matches = true;
                break;
            }
        }

        return matches;
    }

2008/9/13 tzwoenn <sven.li...@googlemail.com>

[Anthony MULLER]

I discover the origin of this problem:

interface ITotoService {
     IToto doSomething();
}

class TotoServiceImpl {
     Toto doSomething() {
        return ..... ;
     }
}

Observe the return type of both methods.

First, the interface, returns IToto.
Second, the implementation returns Toto.

In this case, the invoke() method of a Matcher<Method> class is called twice!

I think Guice should not ask for invocation when a method is "synthetic" ("volatile", "bridge", ... any methods generated by compiler)...

What's your point of view?

Regards,
Anthony MÜLLER

2008/9/15 Anthony MULLER <anthony...@gmail.com>

[limpb...@gmail.com]

On Sep 15, 6:19 am, "Anthony MULLER" <anthony.mul...@gmail.com> wrote:
> interface ITotoService {
>      IToto doSomething();
> }
>
> class TotoServiceImpl {
>      Toto doSomething() {
>         return ..... ;
>      }
> }

Did you intend
class TotoServiceImpl implements ITotoService?

> Observe the return type of both methods.

Is it safe to assume that Toto implements IToto?

> I think Guice should not ask for invocation when a method is "synthetic"
> ("volatile", "bridge", ... any methods generated by compiler)...

I don't think there's any synthetic methods here. Could you
send an executable test case? I think I know what you're
getting at here but it's a lot easier to fix bugs with a test case.

Cheers,
Jesse

[Anthony MULLER]

Yep, TotoServiceImpl implements ITotoService and Toto implements IToto.

I will send you a minimal test case tomorrow...

Cheers,
Anthony

2008/9/15 je...@swank.ca <limpb...@gmail.com>

[Anthony MULLER]

The test case:

@Retention(RUNTIME)
@Target(ElementType.TYPE)
@Inherited
public @interface MyAnno {}


public interface IToto {}


public class TotoImpl implements IToto {}


@MyAnno
public interface ITotoService {

    /**
     * A great method which will do something
     *
     * @return
     */
    IToto doSomething();
}


public class TotoServiceImpl implements ITotoService {

    public TotoImpl doSomething() {
        return null;
    }
}



package tests.guice;

import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;

import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;

import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Injector;
import com.google.inject.matcher.AbstractMatcher;
import com.google.inject.matcher.Matchers;

public class Main {

    public static void main(String[] args) {
        Injector injector = Guice.createInjector(new TotoModule());
        ITotoService totoService = injector.getInstance(ITotoService.class);
       
        // Method call
        totoService.doSomething();
    }
}

class TotoModule extends AbstractModule {

    @Override
    protected void configure() {
        bind(ITotoService.class).to(TotoServiceImpl.class);
        bindInterceptor(Matchers.any(), new TypeAnnotatedMatcher(MyAnno.class), new TotoInterceptor());
    }

}

class TotoInterceptor implements MethodInterceptor {

    public Object invoke(MethodInvocation mi) throws Throwable {
        System.out.println(mi.getMethod().getName() + " -> isSynthetic ? "
                + mi.getMethod().isSynthetic());
        return mi.proceed();
    }
}

class TypeAnnotatedMatcher extends AbstractMatcher<Method> {

    private final Class<? extends Annotation> annClass;

    /**
     * Constructor.
     *
     * @param annClass
     */
    public TypeAnnotatedMatcher(final Class<? extends Annotation> annClass) {
        this.annClass = annClass;

    }

    /**
     * Search for the method signature into all the interface annotated with a
     * given annotation.
     */
    public boolean matches(Method method) {
        // Hook to avoid twice interceptions of the same method

        if (method.isSynthetic()) {
            //return false;

        }

        boolean matches = false;
        Class<?> type = method.getDeclaringClass();
        Class<?>[] iTypes = ReflectUtils.getInterfaces(type, annClass);
        for (Class<?> iType : iTypes) {
            Method iMethod = null;

            try {
                iMethod = iType.getDeclaredMethod(method.getName(), method
                        .getParameterTypes());
            } catch (SecurityException e) {
                // Should never happen
            } catch (NoSuchMethodException e) {
                // Should never happen
            }
            if (iMethod != null) {
                matches = true;
                break;
            }
        }

        return matches;
    }
}

class ReflectUtils {
    /**
     * Get type annoted with the given annotation
     *
     * @param type
     *            Type where we have to search for annoted interfaces
     *
     * @param annClass
     *            Annotation to find
     *
     * @return Class<?>[]
     */
    public final static Class<?>[] getInterfaces(final Class<?> type,
            final Class<? extends Annotation> annClass) {
        List<Class<?>> annTypes = new ArrayList<Class<?>>();
        Class<?>[] iTypes = type.getInterfaces();

        for (Class<?> iType : iTypes) {

            if (iType.isAnnotationPresent(annClass)) {
                annTypes.add(iType);
            }
        }
        return annTypes.toArray(new Class<?>[annTypes.size()]);
    }
}

2008/9/15 Anthony MULLER <anthony...@gmail.com>

[Anthony MULLER]

Hello,

Just a little message to know if you had some time to investigate this issue?

Regards,
Anthony

2008/9/16 Anthony MULLER <anthony...@gmail.com>