groovy
  1. groovy
  2. GROOVY-4832

when two java classes extend a groovy class, both subclasses have a metaClass for whichever subclass was loaded first

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.7.10, 1.8.0
    • Fix Version/s: 1.8.2, 1.9-beta-3, 1.7.11
    • Component/s: None
    • Labels:
      None
    • Environment:
      WinXP
    • Testcase included:
      yes
    • Number of attachments :
      1

      Description

      Original problem:

      I have an abstract groovy class, and two (or more) concrete java subclasses extend it. The superclass has an abstract method, which each concrete subclass overrides (of course).

      When I use just one subclass, everything works fine: I call a superclass method that calls the abstract method, and I get the behavior I expect. But when the other subclass has already been loaded, things are different. Then, I get an IllegalArgumentException: object is not an instance of declaring class.

      This problem corresponds to testGenericSubclassWithBafflingSymptom() in the attached junit tests. See that test for additional details.

      Simpler case that illustrates the likely underlying problem (and does not involve generics):

      I have an abstract groovy class, and two concrete java subclasses extend it. If I have an instance of just one of the subclasses, then instance.metaClass.theClass returns exactly what I expect. But if I've already loaded the other subclass, then the metaClass on an instance of either subclass is for the class that was used first!

      // snippet of the groovy version of testSubclass(), also attached
            OtherConcreteJavaSubclass unrelatedInstance = new OtherConcreteJavaSubclass();
            ConcreteJavaSubclass instance = new ConcreteJavaSubclass();
            assertEquals("this one works", OtherConcreteJavaSubclass, unrelatedInstance.metaClass.theClass)
            assertEquals("but this one is wrong", ConcreteJavaSubclass, instance.metaClass.theClass)
      

      This mixture of groovy and java may sound a little odd, but we actually ran into it when converting an existing class from java to groovy, and it stumped us for quite a while.

        Activity

        Hide
        Alex Heneveld added a comment - - edited

        This bit us as well--as it manifests subtly, because unit tests pass when run in isolation but once both classes are loaded the same code will throw the exception mentioned above:

        java.lang.IllegalArgumentException: object is not an instance of declaring class
        	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
        	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        	at java.lang.reflect.Method.invoke(Method.java:597)
        	at org.codehaus.groovy.runtime.callsite.PogoMetaMethodSite$PogoCachedMethodSite.invoke(PogoMetaMethodSite.java:226)
        	at org.codehaus.groovy.runtime.callsite.PogoMetaMethodSite.callCurrent(PogoMetaMethodSite.java:52)
        	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:145)
        	at instance.methodFromGroovySuperclass
        

        Suggest high priority as this takes a long time to track down and mixing Java and Groovy will be common in a large team.

        The simple workaround we use is to put the following in the groovy superclass constructor (getting the metaclass of the java subclass, curiously, seems to work):

        setMetaClass(DefaultGroovyMethods.getMetaClass(getClass()))
        
        Show
        Alex Heneveld added a comment - - edited This bit us as well--as it manifests subtly, because unit tests pass when run in isolation but once both classes are loaded the same code will throw the exception mentioned above: java.lang.IllegalArgumentException: object is not an instance of declaring class at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.codehaus.groovy.runtime.callsite.PogoMetaMethodSite$PogoCachedMethodSite.invoke(PogoMetaMethodSite.java:226) at org.codehaus.groovy.runtime.callsite.PogoMetaMethodSite.callCurrent(PogoMetaMethodSite.java:52) at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:145) at instance.methodFromGroovySuperclass Suggest high priority as this takes a long time to track down and mixing Java and Groovy will be common in a large team. The simple workaround we use is to put the following in the groovy superclass constructor (getting the metaclass of the java subclass, curiously, seems to work): setMetaClass(DefaultGroovyMethods.getMetaClass(getClass()))
        Hide
        blackdrag blackdrag added a comment -

        closed wrong issue

        Show
        blackdrag blackdrag added a comment - closed wrong issue
        Hide
        blackdrag blackdrag added a comment -

        when I wrote the only place it could be going wrong, the method $getStaticMetaClass from bytecode to Java, I came up with this:

        protected MetaClass $getStaticMetaClass() {
          if (this.getClass()!=A.class) {
            return ScriptBytecodeAdapter.initMetaClass(this)
          } else {
            ClassInfo ci = A.$staticClassInfo
            if (ci==null) {
              ci = ClassInfo.getClassInfo(this.getClass())
              A.$staticClassInfo = ci
            }
            return ci.getMetaClass()
          }
        }
        

        compiling this to bytecode and comparing it with the actual bytecode revealed, that my translation to Java is wrong. In the byteocde

           protected $getStaticMetaClass()Lgroovy/lang/MetaClass;
             ALOAD 0
             INVOKEVIRTUAL java/lang/Object.getClass ()Ljava/lang/Class;
             INVOKESTATIC A.$get$$class$A ()Ljava/lang/Class;
             IF_ACMPNE L0
             ALOAD 0
             INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.initMetaClass (Ljava/lang/Object;)Lgroovy/lang/MetaClass;
             ARETURN
            L0
             GETSTATIC A.$staticClassInfo : Lorg/codehaus/groovy/reflection/ClassInfo;
             ASTORE 1
             ALOAD 1
             IFNONNULL L1
             ALOAD 0
             INVOKEVIRTUAL java/lang/Object.getClass ()Ljava/lang/Class;
             INVOKESTATIC org/codehaus/groovy/reflection/ClassInfo.getClassInfo (Ljava/lang/Class;)Lorg/codehaus/groovy/reflection/ClassInfo;
             DUP
             ASTORE 1
             PUTSTATIC A.$staticClassInfo : Lorg/codehaus/groovy/reflection/ClassInfo;
            L1
             ALOAD 1
             INVOKEVIRTUAL org/codehaus/groovy/reflection/ClassInfo.getMetaClass ()Lgroovy/lang/MetaClass;
             ARETURN
             MAXSTACK = 2
             MAXLOCALS = 2
        

        the IF_ACMPNE should be a IF_ACMPEQ. Doing this change makes the tests in the attached project pass, while it does not without. IF_ACMPNE means we don't do this.getClass()!=A.class, we do this.getClass()==A.class and then it becomes clear that the code above makes no sense like this.

        Show
        blackdrag blackdrag added a comment - when I wrote the only place it could be going wrong, the method $getStaticMetaClass from bytecode to Java, I came up with this: protected MetaClass $getStaticMetaClass() { if ( this .getClass()!=A.class) { return ScriptBytecodeAdapter.initMetaClass( this ) } else { ClassInfo ci = A.$staticClassInfo if (ci== null ) { ci = ClassInfo.getClassInfo( this .getClass()) A.$staticClassInfo = ci } return ci.getMetaClass() } } compiling this to bytecode and comparing it with the actual bytecode revealed, that my translation to Java is wrong. In the byteocde protected $getStaticMetaClass()Lgroovy/lang/MetaClass; ALOAD 0 INVOKEVIRTUAL java/lang/ Object .getClass ()Ljava/lang/ Class ; INVOKESTATIC A.$get$$class$A ()Ljava/lang/ Class ; IF_ACMPNE L0 ALOAD 0 INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.initMetaClass (Ljava/lang/ Object ;)Lgroovy/lang/MetaClass; ARETURN L0 GETSTATIC A.$staticClassInfo : Lorg/codehaus/groovy/reflection/ClassInfo; ASTORE 1 ALOAD 1 IFNONNULL L1 ALOAD 0 INVOKEVIRTUAL java/lang/ Object .getClass ()Ljava/lang/ Class ; INVOKESTATIC org/codehaus/groovy/reflection/ClassInfo.getClassInfo (Ljava/lang/ Class ;)Lorg/codehaus/groovy/reflection/ClassInfo; DUP ASTORE 1 PUTSTATIC A.$staticClassInfo : Lorg/codehaus/groovy/reflection/ClassInfo; L1 ALOAD 1 INVOKEVIRTUAL org/codehaus/groovy/reflection/ClassInfo.getMetaClass ()Lgroovy/lang/MetaClass; ARETURN MAXSTACK = 2 MAXLOCALS = 2 the IF_ACMPNE should be a IF_ACMPEQ. Doing this change makes the tests in the attached project pass, while it does not without. IF_ACMPNE means we don't do this.getClass()!=A.class, we do this.getClass()==A.class and then it becomes clear that the code above makes no sense like this.
        Hide
        blackdrag blackdrag added a comment -

        I corrected the coe as stated before, would be nice if people could test this

        Show
        blackdrag blackdrag added a comment - I corrected the coe as stated before, would be nice if people could test this

          People

          • Assignee:
            blackdrag blackdrag
            Reporter:
            Laura Dean
          • Votes:
            3 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: