Java 8 lambda definitions can break Guice's internal exception handling

[gissuebot]

From abwallis on July 26, 2013 09:10:49

If a @Provides method contains a lambda definition, and an uncaught exception is thrown by the @Provides method, Guice's internal exception handling (UncaughtExceptionHandler) fails with com.google.inject.internal.util.$ComputationException, hiding the underlying cause of the failure and preventing the display of Guice's informative stack trace.
Note that the lambda does not even have to be executed, a user exception can be thrown before the definition of the lambda and will still trigger the failure. Steps to reproduce: 1. Compile and run the attached file.

Guice version: 3.0
JDK version: 1.8.0-ea-b98

Attachment: gist
   Java8LambdaIssue.java

Original issue: http://code.google.com/p/google-guice/issues/detail?id=757

[gissuebot]

From mcculls on July 26, 2013 07:11:30

Note that UncaughtExceptionHandler is actually the JVM's default handler for exceptions not caught in the application, adding a try...catch block around the main method code and printing out the exception reveals:

Exception in thread "main" com.google.inject.internal.util.$ComputationException: java.lang.ArrayIndexOutOfBoundsException: 17665
        at com.google.inject.internal.util.$MapMaker$StrategyImpl.compute(MapMaker.java:553)
        at com.google.inject.internal.util.$MapMaker$StrategyImpl.compute(MapMaker.java:419)
        at com.google.inject.internal.util.$CustomConcurrentHashMap$ComputingImpl.get(CustomConcurrentHashMap.java:2041)
        at com.google.inject.internal.util.$StackTraceElements.forMember(StackTraceElements.java:53)
        at com.google.inject.internal.Errors.formatSource(Errors.java:690)
        at com.google.inject.internal.Errors.format(Errors.java:555)
        at com.google.inject.ProvisionException.getMessage(ProvisionException.java:59)
        at java.lang.Throwable.getLocalizedMessage(Throwable.java:391)
        at java.lang.Throwable.toString(Throwable.java:480)
        at java.lang.String.valueOf(String.java:2985)
        at java.io.PrintStream.println(PrintStream.java:821)
        at java.lang.Throwable$WrappedPrintStream.println(Throwable.java:748)
        at java.lang.Throwable.printStackTrace(Throwable.java:655)
        at java.lang.Throwable.printStackTrace(Throwable.java:643)
        at java.lang.Throwable.printStackTrace(Throwable.java:634)
        at Java8LambdaIssue.main(Java8LambdaIssue.java:16)
Caused by: java.lang.ArrayIndexOutOfBoundsException: 17665
        at com.google.inject.internal.asm.$ClassReader.readClass(Unknown Source)
        at com.google.inject.internal.asm.$ClassReader.accept(Unknown Source)
        at com.google.inject.internal.asm.$ClassReader.accept(Unknown Source)
        at com.google.inject.internal.util.$LineNumbers.<init>(LineNumbers.java:62)
        at com.google.inject.internal.util.$StackTraceElements$1.apply(StackTraceElements.java:36)
        at com.google.inject.internal.util.$StackTraceElements$1.apply(StackTraceElements.java:33)
        at com.google.inject.internal.util.$MapMaker$StrategyImpl.compute(MapMaker.java:549)
        ... 15 more

Can you try with the "no-aop" flavour of Guice 3?  This should avoid the problem as it doesn't use ASM

[gissuebot]

From sberlin on July 26, 2013 07:13:41

Could you give it a shot with the latest version from HEAD?  I'm pretty sure I updated ASM & CGLIB in there, and some of the changes might have fixed this.

[gissuebot]

From mcculls on July 26, 2013 07:19:55

Guice HEAD fails with a slightly different ASM error:

Exception in thread "main" com.google.inject.internal.guava.collect.$ComputationException: java.lang.IllegalArgumentException
        at com.google.inject.internal.guava.collect.$ComputingConcurrentHashMap$ComputingMapAdapter.get(ComputingConcurrentHashMap.java:397)
        at com.google.inject.internal.util.StackTraceElements.forMember(StackTraceElements.java:56)
        at com.google.inject.internal.Errors.formatSource(Errors.java:703)
        at com.google.inject.internal.Errors.format(Errors.java:568)
        at com.google.inject.ProvisionException.getMessage(ProvisionException.java:61)
        at java.lang.Throwable.getLocalizedMessage(Throwable.java:391)
        at java.lang.Throwable.toString(Throwable.java:480)
        at java.lang.String.valueOf(String.java:2985)
        at java.io.PrintStream.println(PrintStream.java:821)
        at java.lang.Throwable$WrappedPrintStream.println(Throwable.java:748)
        at java.lang.Throwable.printStackTrace(Throwable.java:655)
        at java.lang.Throwable.printStackTrace(Throwable.java:643)
        at java.lang.Throwable.printStackTrace(Throwable.java:634)
        at Java8LambdaIssue.main(Java8LambdaIssue.java:16)
Caused by: java.lang.IllegalArgumentException
        at com.google.inject.internal.asm.$ClassReader.<init>(Unknown Source)
        at com.google.inject.internal.asm.$ClassReader.<init>(Unknown Source)
        at com.google.inject.internal.asm.$ClassReader.<init>(Unknown Source)
        at com.google.inject.internal.util.LineNumbers.<init>(LineNumbers.java:65)
        at com.google.inject.internal.util.StackTraceElements$1.apply(StackTraceElements.java:39)
        at com.google.inject.internal.util.StackTraceElements$1.apply(StackTraceElements.java:36)
        at com.google.inject.internal.guava.collect.$ComputingConcurrentHashMap$ComputingValueReference.compute(ComputingConcurrentHashMap.java:355)
        at com.google.inject.internal.guava.collect.$ComputingConcurrentHashMap$ComputingSegment.compute(ComputingConcurrentHashMap.java:184)
        at com.google.inject.internal.guava.collect.$ComputingConcurrentHashMap$ComputingSegment.getOrCompute(ComputingConcurrentHashMap.java:153)
        at com.google.inject.internal.guava.collect.$ComputingConcurrentHashMap.getOrCompute(ComputingConcurrentHashMap.java:69)
        at com.google.inject.internal.guava.collect.$ComputingConcurrentHashMap$ComputingMapAdapter.get(ComputingConcurrentHashMap.java:393)
        ... 13 more

Which might be because Java8 is still a moving target... I haven't tried pulling in the ASM 5 alpha build.

[gissuebot]

From mcculls on July 26, 2013 07:20:59

PS. testing with the "no-aop" flavour of Guice works as expected (ie. the application error is reported properly)

[gissuebot]

From sberlin on July 26, 2013 07:24:05

Yeah, until Java8 stabilizes, I doubt there's much we can do about this.  The error is inside ASM itself, trying to read the class from the bytes of the classfile.  We could potentially put a try/catch around that for IAE, but I'm not a big fan of defensively programming against something that isn't released nor defined.

[gissuebot]

From mcculls on July 26, 2013 07:30:27

The latest ASM 5 code seems to fix the issue.

[gissuebot]

From sameb@google.com on July 26, 2013 07:31:57

Happen to know the timeline for when they expect to ship it?

[gissuebot]

From mcculls on July 26, 2013 07:43:45

Sorry, don't know - http://mail.ow2.org/wws/arc/asm/2013-02/msg00000.html has some related discussion and afaict they're planning to wait for the Java8 spec to finalize before releasing ASM5. There is an early alpha build: http://mail.ow2.org/wws/arc/asm/2013-03/msg00000.html

[gissuebot]

From mcculls on October 22, 2013 16:44:51

FYI using ASM 5.0_BETA with a patched version of cglib (see Issue 759 ) produces the expected stack trace:

Exception in thread "main" com.google.inject.ProvisionException: Guice provision errors:

1. Error in custom provider, java.lang.IllegalStateException: Something went wrong
     at Java8LambdaIssue$LambdaIssueModule.createExecutor(Java8LambdaIssue.java:23)
     while locating java.util.concurrent.Executor

1 error
        at com.google.inject.internal.InjectorImpl$2.get(InjectorImpl.java:1009)
        at com.google.inject.internal.InjectorImpl.getInstance(InjectorImpl.java:1035)
        at Java8LambdaIssue.main(Java8LambdaIssue.java:14)
Caused by: java.lang.IllegalStateException: Something went wrong
        at Java8LambdaIssue$LambdaIssueModule.createExecutor(Java8LambdaIssue.java:24)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:491)
        at com.google.inject.internal.ProviderMethod.get(ProviderMethod.java:105)
        at com.google.inject.internal.ProviderInternalFactory.provision(ProviderInternalFactory.java:86)
        at com.google.inject.internal.InternalFactoryToInitializableAdapter.provision(InternalFactoryToInitializableAdapter.java:55)
        at com.google.inject.internal.ProviderInternalFactory.circularGet(ProviderInternalFactory.java:66)
        at com.google.inject.internal.InternalFactoryToInitializableAdapter.get(InternalFactoryToInitializableAdapter.java:47)
        at com.google.inject.internal.InjectorImpl$2$1.call(InjectorImpl.java:1000)
        at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1050)
        at com.google.inject.internal.InjectorImpl$2.get(InjectorImpl.java:996)
        ... 2 more

If you want to experiment with this solution, it's available on central as sisu-guice 3.1.8: https://github.com/sonatype/sisu-guice # README explains differences wrt. google-guice http://search.maven.org/#artifactdetails%7Corg.sonatype.sisu%7Csisu-guice%7C3.1.8%7Cjar

[gissuebot]

From tebulin on December 04, 2013 06:17:18

Coming from related issue: https://code.google.com/p/google-guice/issues/detail?id=782 > The error is inside ASM itself, trying to read the class from the bytes of the
> classfile.  We could potentially put a try/catch around that for IAE, but I'm
> not a big fan of defensively programming

In my cases ASM only fails for reverse lookups of original source code locations. So on any Guice error I only receive an completely unrelated $ComputedExceptions hiding another unrelated IllegalArgumentException instead of any helpful pointer to the original issue, just because ASM/Guice was unable to resolve the assumed root code line for the exception I do not get any clue about. ;-)

I'd vote for better defense in here, because it's located in the crucial error handling.

Please find attached a small patch against 4.0-beta which makes this part more robust in these error cases and simply omits the line number then.

Attachment: gist
   0001-Do-not-hide-root-exceptions-on-ASM-Java8-errors.patch

[gissuebot]

From sameb@google.com on December 04, 2013 06:23:16

You missed the key part of that quote. :-) ...  "not a big fan of defensively programming against something that isn't released nor defined.". -- Is the java8 spec finalized?

[gissuebot]

From sberlin on December 04, 2013 06:51:46

Issue 782 has been merged into this issue.

[gissuebot]

From tebulin on December 04, 2013 08:02:21

I explicitly left that part out, because in my reasoning I tried to express, that my patch is not about the state of integrated ASM and the Java 8 spec, but Guice robustness on providing meaningful error messages.

In my opinion Guice should not fail on trying to be more precise about an error than needed. Instead it should always fail-safe back to what it is knows about the error instead of hiding it with a totally unrelated subsequent error.

Please have a look at the patch. It's really trivial and I think it's worth to be integrated, because everything else already seems to work for me under Java 8 and I assume the ASM reverse lookup for Errors might be always a candidate for unexpected and flaky failures.

[gissuebot]

From sberlin on December 04, 2013 08:08:49

I respectfully disagree.  Arbitrarily adding try/catch around places just leads to brittle code that can magically succeed or fail without knowing what's going on. Guice (and the libraries it embeds right now) do not expect things to fail in this kind of way, and it is good to blow up when it encounters something that will cause it to fail.   Stuart's suggested changes from earlier are the best approach IMO (either exposing the problem more transparently, or solving it entirely).

[gissuebot]

From mingfai.ma on December 27, 2013 21:37:42

tried a number of combination and finally get Guice to show error message for Java 8 Lambda properly :

1. Build a custom Guice snapshot with Java 8
   1.1 update core/pom.xml
         update to the latest cglib and asm
    <properties>
       <cglib.version>3.1</cglib.version>
       <asm.version>5.0_BETA</asm.version>
     </properties>
         comment maven-javadoc-plugin and animal-sniffer-maven-plugin for building with Java 8

1.2 update extensions/pom.xml
      comment maven-javadoc-plugin and animal-sniffer-maven-plugin for building with Java 8

1.2 build Guice
    mvn clean compile package source:jar install -pl .,core,extensions,extensions/assistedinject,extensions/multibindings  -DskipTests
notice that

• the above command build two of the extensions
• assistedinject actually got test failure so something may be broken.

1. Configure the pom.xml in your project to include javax.inject, incl. Guice with no_aop classifier
    <dependency>
               <groupId>javax.inject</groupId>
               <artifactId>javax.inject</artifactId>
               <version>1</version>
           </dependency>
           <dependency>
               <groupId>com.google.inject</groupId>
               <artifactId>guice</artifactId>
               <version>${guice.version}</version>
               <classifier>no_aop</classifier>
           </dependency>

Some of the steps/configuration may not be needed. At least, it is one of the ways that make it work.