[jira] [Closed] (GROOVY-8229) nested try-catch-finally handling inside Closures generates wrong bytecode

["Daniel Sun (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/GROOVY-8229?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Daniel Sun closed GROOVY-8229.
------------------------------

> nested try-catch-finally handling inside Closures generates wrong bytecode
> --------------------------------------------------------------------------
>
>                 Key: GROOVY-8229
>                 URL: https://issues.apache.org/jira/browse/GROOVY-8229
>             Project: Groovy
>          Issue Type: Bug
>          Components: bytecode
>    Affects Versions: 2.4.10, 2.4.11
>            Reporter: Leonard Brünings
>            Assignee: Daniel Sun
>            Priority: Critical
>             Fix For: 2.4.12
>
>
> Since version 2.4.10 groovy generates the wrong bytecode instructions for try-catch-finally inside closures. This is most likely caused by one of these two changes: GROOVY-8085, GROOVY-7248.
> This was discovered with the generated code for Spocks {{verifyAll}} feature, here is the corresponding issue https://github.com/spockframework/spock/issues/709
> The problem can be observed without Spock in this scenario ({{@CompileStatic}} is only used to generate better readable decompiled code, the problem is the same with dynamic code).
> {code:java}
> import groovy.transform.CompileStatic
> import org.junit.Assert
> import org.junit.Test
> import org.junit.runners.model.MultipleFailureException
> @CompileStatic
> class TryCatchProblem {
>   @Test
>   void "test"() {
>     def cl = {
>       ErrorCollector collector = new ErrorCollector()
>       try {
>         try {
>           Assert.fail("a")
>         } catch (Throwable e) {
>           collector.collect(e)
>         }
>         try {
>           Assert.fail("b")
>         } catch (Throwable e) {
>           collector.collect(e)
>         }
>         try {
>           Assert.assertTrue("c", true)
>         } catch (Throwable e) {
>           collector.collect(e)
>         }
>       } finally {
>         collector.check()
>       }
>     }
>     cl()
>   }
> }
> class ErrorCollector {
>   List<Throwable> errors = []
>   int ctr = 0
>   void collect(Throwable t) {
>     errors.add(t)
>   }
>   void check() {
>     if (!errors.empty) {
>       List<Throwable> x = new ArrayList<>(errors)
>       x << new RuntimeException("ctr ${++ctr}")
>       throw new MultipleFailureException(x)
>     }
>   }
> }
> {code}
> The different results can be seen here: https://gist.github.com/leonard84/23f072b2c434b27e851a3e513570acf1
> {panel:title=Result-2.4.9}
> java.lang.AssertionError: a
> java.lang.AssertionError: b
> java.lang.RuntimeException: ctr 1
> {panel}
> {panel:title=Result-2.4.10}
> java.lang.AssertionError: a
> java.lang.AssertionError: b
> java.lang.AssertionError: a
> java.lang.AssertionError: b
> java.lang.RuntimeException: ctr 1
> java.lang.RuntimeException: ctr 3
> {panel}
> And the decompiled code can be seen here: https://gist.github.com/leonard84/1c91f8aebd0b8af599a6925404e5a11c
> The relevant part is here:
> {code:java|title=2.4.9}
>             try {
>                 Assert.assertTrue("c", true);
>                 final Object o = null;
>                 collector.check();
>                 return o;
>             }
>             catch (Throwable e3) {
>                 collector.collect(e3);
>                 final Object o2 = null;
>                 collector.check();
>                 return o2;
>             }
>             collector.check();
>         }
>         finally {
>             collector.check();
>         }
>         return null;
> }
> {code}
> vs.
> {code:java|title=2.4.10}
>             Object o = null;
>             try {
>                 Assert.assertTrue("c", true);
>                 o = null;
>                 try {
>                     final ErrorCollector errorCollector = collector;
>                     errorCollector.check();
>                     return o;
>                 }
>                 catch (Throwable e3) {
>                     collector.collect(e3);
>                     final Object o3 = null;
>                     collector.check();
>                     return o3;
>                 }
>             }
>             catch (Throwable t) {}
>             try {
>                 final ErrorCollector errorCollector = collector;
>                 errorCollector.check();
>                 return o;
>             }
>             finally {}
>             collector.check();
>         }
>         finally {
>             collector.check();
>         }
>         return null;
> }
> {code}
> In both cases too many calls to {{collector.check()}} are generated, however since the finally block was not executed in <groovy-2.4.10 it was not really an issue. In the 2.4.10 code the generated code is definitely wrong. Somehow a call to {{collector.check()}} was put in its own try-catch, which throws the first exception which is then catched, appended and thrown inside a new exception with the call to {{collector.check()}} inside the catch. This Exception is then overridden by the call to {{collector.check()}} in the finally block, as you can see by the {{ctr 3}} instead of {{ctr 2}} inside the second RuntimeException.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)