[weld-dev] Bridge methods & proxies & interceptors

[Stuart Douglas]

I think so. Not sure how this affects Glassfish though.

Stuart

Ales Justin wrote:
> So this means we can drop Marko' fix?
>
> On Sep 25, 2012, at 2:24, Stuart Douglas<stuart.w.douglas at gmail.com>  wrote:
>
>> This is the relevant AS7 fix:
>>
>> https://github.com/jbossas/jboss-as/pull/3112
>>
>> Stuart
>>
>> Marko Lukša wrote:
>>> Hey
>>>
>>> I need someone to take a look at https://github.com/weld/core/pull/225
>>>
>>> My initial change was to add bridge methods to the proxy (when proxying
>>> an interface), but later realized this could never work, because the
>>> interceptors expect a Method object that would have to point to a
>>> non-existing method (the method exists only on the proxy, but not on the
>>> interface).
>>>
>>> That's why the only solution was to make Beans.getInterceptableMethods()
>>> return the bridge methods also. The reason why you guys had problems
>>> with this in the past was the fact that MethodReference was missing the
>>> return type and thus treated two methods with the same name and
>>> parameters but different return types as equal, when they clearly
>>> aren't. This is why Weld was throwing the duplicate interceptor
>>> definition errors.
>>>
>>> Marko
>>>
>>> On 7.9.2012 17:43, Marko Lukša wrote:
>>>> Yes, when the proxy extends a class, everything is taken care of by
>>>> the bridge method in the superclass. But, when dealing with local EJB
>>>> interfaces, the proxy's superclass is Object, so there is no bridge
>>>> method at all.
>>>>
>>>> For EJBs, the stacktrace would look something like this:
>>>>     StringFooImpl.doSth(String)
>>>>     StringFooImpl.doSth(Object)
>>>>     methodHandler.invoke("doSth(Object)")
>>>>     StringFoo$Proxy.doSth(Object)<-- proxy extends Object, implements
>>>> StringFoo interface (no bridge method)
>>>>
>>>> For regular managed beans, the stacktrace would look like this:
>>>>     StringFooImpl.doSth(String)
>>>>     methodHandler.invoke("doSth(String)")
>>>>     StringFooImpl$Proxy.doSth(String)
>>>>     StringFooImpl$Proxy.doSth(Object)<-- proxy extends StringFooImpl
>>>> (and has bridge method)
>>>>
>>>> Note the different methods methodHandler is handling.
>>>>
>>>> Marko
>>>>
>>>>
>>>> On 7.9.2012 17:27, Stuart Douglas wrote:
>>>>> IMHO the correct way to deal with this is to simply make bridge
>>>>> methods delegate to super(), which will then result in the actual
>>>>> intercepted method being called.
>>>>>
>>>>> To be honest I thought we already did this, as we have had multiple
>>>>> related bugs in the past. Do you have a test case that I can look at?
>>>>>
>>>>> Stuart
>>>>>
>>>>>> Marko Lukša<mailto:marko.luksa at gmail.com>
>>>>>> 7 September 2012 7:19 PM
>>>>>> Hey all.
>>>>>>
>>>>>> I've been working on https://issues.jboss.org/browse/WELD-1162 and
>>>>>> need your opinion.
>>>>>>
>>>>>> Say we have:
>>>>>>
>>>>>> public interface Foo<T>  {
>>>>>>    void doSomething(T t);
>>>>>> }
>>>>>> public interface StringFoo extends Foo<String>  {}
>>>>>> public class StringFooImpl implements StringFoo {}
>>>>>>
>>>>>> and
>>>>>>
>>>>>> @Inject StringFoo stringFoo;
>>>>>>
>>>>>> The proxy created by Weld is a subclass of StringFooImpl and
>>>>>> therefore has two declared methods:
>>>>>>
>>>>>> void doSomething(Object o) { doSomething((String) o); }
>>>>>> void doSomething(String) {...}
>>>>>>
>>>>>> However, when StringFooImpl is a session bean, with StringFoo as its
>>>>>> local interface, the proxy is a subclass of Object and therefore the
>>>>>> proxy only has the following declared method:
>>>>>>
>>>>>> void doSomething(Object o);
>>>>>>
>>>>>> In both cases, when a client invokes stringFoo.doSomething("foo"),
>>>>>> the method doSomething(Object) is invoked. But there's a difference
>>>>>> in what happens next:
>>>>>>
>>>>>>   * In the non-ejb version, the bridge method then immediately
>>>>>>     invokes doSomething(String) and only then is the proxy's method
>>>>>>     handler invoked. The handler is therefore dealing with the
>>>>>>     method doSomething(*String*)
>>>>>>   * in the EJB version, doSomething(Object) is not a bridge method,
>>>>>>     and so the method handler is invoked directly and it (the
>>>>>>     handler) is operating on doSomething(*Object*).
>>>>>>
>>>>>> In the second case, this ultimately means that Weld will check
>>>>>> whether doSomething(Object) is intercepted. It isn't, since
>>>>>> Beans.getInterceptableMethods() is ignoring bridge methods. The
>>>>>> interceptor will not be invoked. On the other hand, in the first
>>>>>> case, the interceptor _will_ be invoked, since Weld will be checking
>>>>>> whether doSomething(String) is intercepted.
>>>>>>
>>>>>> My initial solution was to make Beans.getInterceptableMethods() also
>>>>>> return bridge methods, but now I'm thinking the actual problem is in
>>>>>> the proxy itself. IMO, when creating a proxy based on an interface,
>>>>>> we should also generate bridge methods on the proxy (this should be
>>>>>> either done by Weld or by Javassist directly). These bridge methods
>>>>>> should be perfectly normal bridge methods and should not invoke the
>>>>>> method handler directly. They should simply invoke the non-bridge
>>>>>> method and the non-bridge method should then invoke the method handler.
>>>>>>
>>>>>> The java compiler can't add bridge methods directly to interfaces
>>>>>> which require them, so it adds them to all the classes implementing
>>>>>> the interface (StringFooImpl in our case). Since we are creating
>>>>>> StringFoo$Proxy, which is also a class implementing an interface
>>>>>> which requires bridge methods, we should add the bridge methods
>>>>>> to it - exactly as the java compiler would.
>>>>>>
>>>>>> This would solve the interceptor problem and possibly other similar
>>>>>> problems as well.
>>>>>>
>>>>>> What do you think?
>>>>>>
>>>>>> Marko
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> weld-dev mailing list
>>>>>> weld-dev at lists.jboss.org
>>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev
>>> _______________________________________________
>>> weld-dev mailing list
>>> weld-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/weld-dev
>> _______________________________________________
>> weld-dev mailing list
>> weld-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/weld-dev