[jboss-jira] [JBoss JIRA] (JASSIST-189) Bridge methods are proxied instead of desired methods

[Donnchadh Ó Donnabháin (JIRA)]

    [ https://issues.jboss.org/browse/JASSIST-189?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12743976#comment-12743976 ] 

Donnchadh Ó Donnabháin commented on JASSIST-189:
------------------------------------------------

The first {{hash.put(key, oldMethod)}} is part of the existing code to prevent (or rather undo) replacing a method with with one of lower visibility.

The new code in the patch undoes the situation where we have just replaced a non-bridge method with a bridge method.
The idea is that if we are going to pick one method, it should be the non-bridge method since the bridge method will call the non-bridge method in any case.

The following is the entire patched method:
{code:title=ProxyFactory.java}
    private static void getMethods(HashMap hash, Class clazz, Set visitedClasses) {
        // This both speeds up scanning by avoiding duplicate interfaces and is needed to
        // ensure that superinterfaces are always scanned before subinterfaces.
        if (!visitedClasses.add(clazz))
            return;

        Class[] ifs = clazz.getInterfaces();
        for (int i = 0; i < ifs.length; i++)
            getMethods(hash, ifs[i], visitedClasses);

        Class parent = clazz.getSuperclass();
        if (parent != null)
            getMethods(hash, parent, visitedClasses);

        Method[] methods = SecurityActions.getDeclaredMethods(clazz);
        for (int i = 0; i < methods.length; i++)
            if (!Modifier.isPrivate(methods[i].getModifiers())) {
                Method m = methods[i];
                // JIRA JASSIST-127 (covariant return types).
                String key = m.getName() + ':' + RuntimeSupport.makeDescriptor(m.getParameterTypes(), null);
                // JIRA JASSIST-85
                // put the method to the cache, retrieve previous definition (if any) 
                Method oldMethod = (Method)hash.put(key, methods[i]); 

                // check if visibility has been reduced 
                if (null != oldMethod && Modifier.isPublic(oldMethod.getModifiers())
                                      && !Modifier.isPublic(methods[i].getModifiers()) ) { 
                    // we tried to overwrite a public definition with a non-public definition,
                    // use the old definition instead. 
                    hash.put(key, oldMethod); 
                } else if (null != oldMethod && !oldMethod.isBridge() && methods[i].isBridge() 
                        && oldMethod.getDeclaringClass() == methods[i].getDeclaringClass()) {
                    hash.put(key, oldMethod); 
                }
            }
    }
{code}
                
> Bridge methods are proxied instead of desired methods
> -----------------------------------------------------
>
>                 Key: JASSIST-189
>                 URL: https://issues.jboss.org/browse/JASSIST-189
>             Project: Javassist
>          Issue Type: Bug
>    Affects Versions: 3.17.1-GA
>         Environment: OS X, Java 1.7.0_09
>            Reporter: Donnchadh Ó Donnabháin
>            Assignee: Shigeru Chiba
>
> The following test intermittently fails:
> {code}
> package org.javassist.test.covariantproxy;
> import java.lang.reflect.Method;
> import javassist.util.proxy.MethodHandler;
> import javassist.util.proxy.ProxyFactory;
> import javassist.util.proxy.ProxyObject;
> import junit.framework.TestCase;
> public class CovariantProxyTest extends TestCase {
>     
>     public interface TestProxy {
>         
>     }
>     
>     public static class TestMethodHandler implements MethodHandler {
>         
>         boolean invoked = false;
>         public Object invoke(Object self, Method thisMethod, Method proceed, Object[] args) throws Throwable {
>             invoked = true;
>             return proceed.invoke(self, args);
>         }
>         
>         public boolean wasInvoked() {
>             return invoked;
>         }
>         public void reset() {
>             invoked = false;
>         }
>     }
>     public static class Issue {
>     
>         private Integer id;
>     
>         public Integer getId() {
>             return id;
>         }
>     
>         public void setId(Integer id) {
>             this.id = id;
>         }
>     }
>     
>     public static class PublishedIssue extends Issue {
>     
>     }
>     public static abstract class Article {
>         
>         private Integer id;
>     
>         public Integer getId() {
>             return id;
>         }
>     
>         public void setId(Integer id) {
>             this.id = id;
>         }
>         
>         public abstract Issue getIssue();
>     }
>     public static class PublishedArticle extends Article {
>     
>         private PublishedIssue issue;
>         
>         @Override
>         public PublishedIssue getIssue() {
>             return issue;
>         }
>     
>         public void setIssue(PublishedIssue issue) {
>             this.issue = issue;
>         }
>         
>     }
>     
>     public void testThatCallingAMethodWithCovariantReturnTypeCallsProxy() throws Exception {
>         Class persistentClass = PublishedArticle.class;
>         ProxyFactory factory = new ProxyFactory();
>         factory.setUseCache(false);
>         factory.setSuperclass(persistentClass);
>         factory.setInterfaces(new Class[] {TestProxy.class});
>         Class cl = factory.createClass();
>         TestProxy proxy = ( TestProxy ) cl.newInstance();
>         TestMethodHandler methodHandler = new TestMethodHandler();
>         ( ( ProxyObject ) proxy ).setHandler( methodHandler );
>         
>         ((Article)proxy).getIssue();
>         assertTrue(methodHandler.wasInvoked());
>         
>         methodHandler.reset();
>         
>         PublishedArticle article = (PublishedArticle) proxy;
>         
>         article.getIssue();
>         
>         assertTrue(methodHandler.wasInvoked());
>         
>     }
>     
> }
> {code}
> Were there tests added for JASSIST-24 ? I don't see any associated source.
> This bug causes a problem in hibernate, described in  https://hibernate.onjira.com/browse/HHH-7884 .
> This may be related to JASSIST-162 but the circumstances are slightly different.
> The problem occurs intermittently so it appears to depend on the order in which the methods are processed.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira