Uploaded image for project: 'Groovy'
  1. Groovy
  2. GROOVY-7826

Infinite recursion in genericTypeAsString

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.0-beta-2
    • Fix Version/s: 2.4.7
    • Component/s: None
    • Labels:
    • Environment:
      Groovy Version: 2.4.6 JVM: 1.8.0_91 Vendor: Oracle Corporation OS: Linux

      Description

      The following two Java classes C1 and C2 cause Groovy to enter infinite recursion in genericTypeAsString when a method that takes a C1 is declared:

      C1.java:

      public class C1 <T2 extends C2<T2,T1>,T1 extends C1<T2,T1>> { }
      class C2<T2 extends C2<T2, T1>, T1 extends C1<T2, T1>> { }
      

      repro.groovy

      def f(C1 c1) { }
      

      This is reduced from actual code in Jenkins, where Run and Job have type parameters like this.

      1. groovy-7826.zip
        0.8 kB
        Magnus Reftel

        Issue Links

          Activity

          Hide
          blackdrag Jochen Theodorou added a comment -

          see also GROOVY-7840

          Show
          blackdrag Jochen Theodorou added a comment - see also GROOVY-7840
          Hide
          daniel.spilker@hamburg.de Daniel Spilker added a comment -

          The problem has been (indirectly) introduced in 2.2.0-beta-2 by commit 74089c1.

          Can someone please update the "Affects Version" field?

          Show
          daniel.spilker@hamburg.de Daniel Spilker added a comment - The problem has been (indirectly) introduced in 2.2.0-beta-2 by commit 74089c1 . Can someone please update the "Affects Version" field?
          Hide
          pascalschumacher Pascal Schumacher added a comment -

          Pull request merged. Thanks!

          Show
          pascalschumacher Pascal Schumacher added a comment - Pull request merged. Thanks!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/groovy/pull/333

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/groovy/pull/333
          Hide
          blackdrag Jochen Theodorou added a comment -

          thanks for taking the time to explain the problem in more detail. Actually I was misunderstanding something and not really understanding the issue. But the recursive use of C2 is indeed something that can be solved with your patch. And since we need to break the toString somewhere I think you have chosen a good place for that.

          Show
          blackdrag Jochen Theodorou added a comment - thanks for taking the time to explain the problem in more detail. Actually I was misunderstanding something and not really understanding the issue. But the recursive use of C2 is indeed something that can be solved with your patch. And since we need to break the toString somewhere I think you have chosen a good place for that.
          Hide
          daniel.spilker@hamburg.de Daniel Spilker added a comment -

          DecompiledClassNode should be fixed if that behavior is not expected. But ClassNode#toString needs to fixed in any case.

          What is bad about the fix in PR #333? Are the redirects for type arguments really important for #toString? What is the expected behavior for #toString?

          This is the computed value with the fix:

          C1 -> C1 <T2 extends C2 <T2, T1>, T1 extends C1>
          

          This is the computed value without the fix and with ASM resolving enabled:

          C1 -> C1 <T2 extends C2 <T2, T1> -> C2, T1 extends C1>
          

          Without the fix and without ASM resolving, the computation never stops:

          C1 -> C1 <T2 extends C2 <T2, T1> -> C2 <T2 extends C2, T1 extends C1 <T2, T1> -> C1 <T2 extends C2 <T2, T1> -> C2 <T2 extends C2, T1 extends C1 <T2, T1> -> ...
          

          This is the same ouput as above without the fix and with ASM resolving enabled, but I added (class.name@identityHashCode) to the type name to show the instances:

          C1(org.codehaus.groovy.ast.ClassNode@12e335ef) -> C1(org.codehaus.groovy.ast.decompiled.DecompiledClassNode@2e9e799) <T2 extends C2(org.codehaus.groovy.ast.ClassNode@4a63ef4d) <T2, T1> -> C2(org.codehaus.groovy.ast.decompiled.DecompiledClassNode@66f3ce62), T1 extends C1>
          

          ... and with ASM resolving disabled:

          C1(org.codehaus.groovy.ast.ClassNode@7c91f442) -> C1(org.codehaus.groovy.ast.ClassNode@523b58f2) <T2 extends C2(org.codehaus.groovy.ast.ClassNode@364ca20b) <T2, T1> -> C2(org.codehaus.groovy.ast.ClassNode@61229c06) <T2 extends C2, T1 extends C1(org.codehaus.groovy.ast.ClassNode@27317fbc) <T2, T1> -> C1(org.codehaus.groovy.ast.ClassNode@523b58f2) <T2 ...
          

          PS: I removed the org.codehaus.groovy.ast.Groovy7862Bug$ prefix for the names of C1 and C2 for brevity.

          PPS: Shouldn't the expected value should be this?

          C1 -> C1<T2 extends C2<T2, T1>, T1 extends C1<T2, T1>>
          
          Show
          daniel.spilker@hamburg.de Daniel Spilker added a comment - DecompiledClassNode should be fixed if that behavior is not expected. But ClassNode#toString needs to fixed in any case. What is bad about the fix in PR #333? Are the redirects for type arguments really important for #toString? What is the expected behavior for #toString? This is the computed value with the fix: C1 -> C1 <T2 extends C2 <T2, T1>, T1 extends C1> This is the computed value without the fix and with ASM resolving enabled: C1 -> C1 <T2 extends C2 <T2, T1> -> C2, T1 extends C1> Without the fix and without ASM resolving, the computation never stops: C1 -> C1 <T2 extends C2 <T2, T1> -> C2 <T2 extends C2, T1 extends C1 <T2, T1> -> C1 <T2 extends C2 <T2, T1> -> C2 <T2 extends C2, T1 extends C1 <T2, T1> -> ... This is the same ouput as above without the fix and with ASM resolving enabled, but I added (class.name@identityHashCode) to the type name to show the instances: C1(org.codehaus.groovy.ast.ClassNode@12e335ef) -> C1(org.codehaus.groovy.ast.decompiled.DecompiledClassNode@2e9e799) <T2 extends C2(org.codehaus.groovy.ast.ClassNode@4a63ef4d) <T2, T1> -> C2(org.codehaus.groovy.ast.decompiled.DecompiledClassNode@66f3ce62), T1 extends C1> ... and with ASM resolving disabled: C1(org.codehaus.groovy.ast.ClassNode@7c91f442) -> C1(org.codehaus.groovy.ast.ClassNode@523b58f2) <T2 extends C2(org.codehaus.groovy.ast.ClassNode@364ca20b) <T2, T1> -> C2(org.codehaus.groovy.ast.ClassNode@61229c06) <T2 extends C2, T1 extends C1(org.codehaus.groovy.ast.ClassNode@27317fbc) <T2, T1> -> C1(org.codehaus.groovy.ast.ClassNode@523b58f2) <T2 ... PS: I removed the org.codehaus.groovy.ast.Groovy7862Bug$ prefix for the names of C1 and C2 for brevity. PPS: Shouldn't the expected value should be this? C1 -> C1<T2 extends C2<T2, T1>, T1 extends C1<T2, T1>>
          Hide
          blackdrag Jochen Theodorou added a comment -

          I feel bad about the change in PR #333. If DecompiledClassNode misses the generic type information it is actually a bug, unless the information is invalid, then this needs to be fixed. if the information is valid, then of course it should not cause a problem. Comparing the generics information when C1 is in Java and when it is in Groovy should reveal the problem.

          Show
          blackdrag Jochen Theodorou added a comment - I feel bad about the change in PR #333. If DecompiledClassNode misses the generic type information it is actually a bug, unless the information is invalid, then this needs to be fixed. if the information is valid, then of course it should not cause a problem. Comparing the generics information when C1 is in Java and when it is in Groovy should reveal the problem.
          Hide
          daniel.spilker@hamburg.de Daniel Spilker added a comment -

          See PR #333 for a reproducer. As far as I could see in the debugger, on master the ClassNodes of the generic type bounds are org.codehaus.groovy.ast.decompiled.DecompiledClassNode which do not carry the generic type information. And that breaks the recursion. When disabling ASM resolving, the problem can be reproduced on master.

          It would be great if this could be fixed in 2.4. I backported my fix to 2.4 and testing this Jenkins (JENKINS-34751) has been successful.

          Show
          daniel.spilker@hamburg.de Daniel Spilker added a comment - See PR #333 for a reproducer. As far as I could see in the debugger, on master the ClassNodes of the generic type bounds are org.codehaus.groovy.ast.decompiled.DecompiledClassNode which do not carry the generic type information. And that breaks the recursion. When disabling ASM resolving, the problem can be reproduced on master. It would be great if this could be fixed in 2.4. I backported my fix to 2.4 and testing this Jenkins ( JENKINS-34751 ) has been successful.
          Hide
          paulk Paul King added a comment - - edited

          I also get the problem on the 2_4_X branch but not on master. Needs further investigation to see if some fix has already been applied and whether the fix can be backported onto the 2_4_X branch. I haven't done a git bisect as yet

          Show
          paulk Paul King added a comment - - edited I also get the problem on the 2_4_X branch but not on master. Needs further investigation to see if some fix has already been applied and whether the fix can be backported onto the 2_4_X branch. I haven't done a git bisect as yet
          Hide
          blackdrag Jochen Theodorou added a comment -

          it is not really a regression since 2,2.0 was the first version to not to ignore that code. Still has to be fixed

          Show
          blackdrag Jochen Theodorou added a comment - it is not really a regression since 2,2.0 was the first version to not to ignore that code. Still has to be fixed
          Hide
          magnus.reftel Magnus Reftel added a comment -

          Tested with a couple of different versions, and it seems like the problem was introduced in 2.2.0-beta-2, as all tested versions up to and including 2.2.0-beta-1 works.

          Log follows:

          $ for a in ~/opt/groovy-*; do echo `basename $a`; $a/bin/groovy -cp . repro.groovy; done
          groovy-1.8.9
          groovy-2.1.9
          groovy-2.2.0
          Caught: java.lang.StackOverflowError
          java.lang.StackOverflowError
          groovy-2.2.0-beta-1
          groovy-2.2.0-beta-2
          Caught: java.lang.StackOverflowError
          java.lang.StackOverflowError
          groovy-2.2.0-rc-1
          Caught: java.lang.StackOverflowError
          java.lang.StackOverflowError
          groovy-2.2.0-rc-3
          Caught: java.lang.StackOverflowError
          java.lang.StackOverflowError
          groovy-2.2.1
          Caught: java.lang.StackOverflowError
          java.lang.StackOverflowError
          groovy-2.2.2
          Caught: java.lang.StackOverflowError
          java.lang.StackOverflowError
          groovy-2.3.11
          Caught: java.lang.StackOverflowError
          java.lang.StackOverflowError
          groovy-2.4.6
          Caught: java.lang.StackOverflowError
          java.lang.StackOverflowError
          $

          Show
          magnus.reftel Magnus Reftel added a comment - Tested with a couple of different versions, and it seems like the problem was introduced in 2.2.0-beta-2, as all tested versions up to and including 2.2.0-beta-1 works. Log follows: $ for a in ~/opt/groovy-*; do echo `basename $a`; $a/bin/groovy -cp . repro.groovy; done groovy-1.8.9 groovy-2.1.9 groovy-2.2.0 Caught: java.lang.StackOverflowError java.lang.StackOverflowError groovy-2.2.0-beta-1 groovy-2.2.0-beta-2 Caught: java.lang.StackOverflowError java.lang.StackOverflowError groovy-2.2.0-rc-1 Caught: java.lang.StackOverflowError java.lang.StackOverflowError groovy-2.2.0-rc-3 Caught: java.lang.StackOverflowError java.lang.StackOverflowError groovy-2.2.1 Caught: java.lang.StackOverflowError java.lang.StackOverflowError groovy-2.2.2 Caught: java.lang.StackOverflowError java.lang.StackOverflowError groovy-2.3.11 Caught: java.lang.StackOverflowError java.lang.StackOverflowError groovy-2.4.6 Caught: java.lang.StackOverflowError java.lang.StackOverflowError $
          Hide
          magnus.reftel Magnus Reftel added a comment -

          This is a regression, as it works fine with

          Groovy Version: 1.8.6 JVM: 1.8.0_31 Vendor: Oracle Corporation OS: Linux

          Show
          magnus.reftel Magnus Reftel added a comment - This is a regression, as it works fine with Groovy Version: 1.8.6 JVM: 1.8.0_31 Vendor: Oracle Corporation OS: Linux

            People

            • Assignee:
              pascalschumacher Pascal Schumacher
              Reporter:
              magnus.reftel Magnus Reftel
            • Votes:
              1 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development