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

[GoogleCodeExporter]

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

Original issue reported on code.google.com by abwal...@gmail.com on 26 Jul 2013 at 1:10

Attachments:

• Java8LambdaIssue.java

[GoogleCodeExporter]

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

Original comment by mccu...@gmail.com on 26 Jul 2013 at 2:11

[GoogleCodeExporter]

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.

Original comment by sberlin on 26 Jul 2013 at 2:13

[GoogleCodeExporter]

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.

Original comment by mccu...@gmail.com on 26 Jul 2013 at 2:19

[GoogleCodeExporter]

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

Original comment by mccu...@gmail.com on 26 Jul 2013 at 2:20

[GoogleCodeExporter]

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.

Original comment by sberlin on 26 Jul 2013 at 2:24

[GoogleCodeExporter]

The latest ASM 5 code seems to fix the issue.

Original comment by mccu...@gmail.com on 26 Jul 2013 at 2:30

[GoogleCodeExporter]

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

Original comment by sa...@google.com on 26 Jul 2013 at 2:31

[GoogleCodeExporter]

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

Original comment by mccu...@gmail.com on 26 Jul 2013 at 2:43

[GoogleCodeExporter]

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

Original comment by mccu...@gmail.com on 22 Oct 2013 at 11:44

[GoogleCodeExporter]

Coming from related issue: 
http://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.

Original comment by tebu...@googlemail.com on 4 Dec 2013 at 2:17

Attachments:

• 0001-Do-not-hide-root-exceptions-on-ASM-Java8-errors.patch

[GoogleCodeExporter]

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?  

Original comment by sa...@google.com on 4 Dec 2013 at 2:23

[GoogleCodeExporter]

Issue 782 has been merged into this issue.

Original comment by sberlin on 4 Dec 2013 at 2:51

[GoogleCodeExporter]

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.

Original comment by tebu...@googlemail.com on 4 Dec 2013 at 4:02

[GoogleCodeExporter]

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).

Original comment by sberlin on 4 Dec 2013 at 4:08

[GoogleCodeExporter]

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. 

2. 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.

Original comment by mingfai...@gmail.com on 28 Dec 2013 at 5:37

[GoogleCodeExporter]

This problem, or a very similar one, exists still in 4.0beta4 which claims Java 
8 compatibility:

Exception in thread "main" 
com.google.common.util.concurrent.UncheckedExecutionException: 
java.lang.RuntimeException
    at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2201)
    at com.google.common.cache.LocalCache.get(LocalCache.java:3934)
    at com.google.common.cache.LocalCache.getOrLoad(LocalCache.java:3938)
    at com.google.common.cache.LocalCache$LocalLoadingCache.get(LocalCache.java:4821)
    at com.google.common.cache.LocalCache$LocalLoadingCache.getUnchecked(LocalCache.java:4827)
    at com.google.inject.internal.util.StackTraceElements.forMember(StackTraceElements.java:66)
    at com.google.inject.internal.Errors.formatInjectionPoint(Errors.java:790)
    at com.google.inject.internal.Errors.formatSource(Errors.java:752)
    at com.google.inject.internal.Errors.formatSource(Errors.java:743)
    at com.google.inject.internal.Errors.format(Errors.java:569)
    at com.google.inject.CreationException.getMessage(CreationException.java:50)
    at java.lang.Throwable.getLocalizedMessage(Throwable.java:391)
    at java.lang.Throwable.toString(Throwable.java:480)
    at java.lang.String.valueOf(String.java:2979)
    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 com.opentable.service.discovery.server.DiscoveryServerMain.main(DiscoveryServerMain.java:61)
Caused by: java.lang.RuntimeException
    at com.google.inject.internal.asm.$MethodVisitor.visitParameter(Unknown Source)
    at com.google.inject.internal.asm.$ClassReader.b(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:65)
    at com.google.inject.internal.util.StackTraceElements$1.load(StackTraceElements.java:46)
    at com.google.inject.internal.util.StackTraceElements$1.load(StackTraceElements.java:43)
    at com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache.java:3524)
    at com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2317)
    at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2280)
    at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2195)
    ... 19 more

Original comment by ste...@trumpet.io on 1 Apr 2014 at 5:02

[GoogleCodeExporter]

Can you whittle this down to a small reproducible test-case, so we can see 
what's causing it?

Original comment by sberlin on 1 Apr 2014 at 5:08

[GoogleCodeExporter]

Here's a test case, requires Java 8 obviously:

https://gist.github.com/stevenschlansker/9981948

Original comment by ste...@trumpet.io on 4 Apr 2014 at 7:56

[GoogleCodeExporter]

Note that changing the ASM level from Opcodes.ASM4 -> ASM5 seems to fix the 
problem.

Original comment by ste...@trumpet.io on 4 Apr 2014 at 11:06

[GoogleCodeExporter]

Well, that's frustrating.

Original comment by sberlin on 4 Apr 2014 at 11:42

[GoogleCodeExporter]

Tell me about it!  :P
Would love for this to get fixed for 4.0 release.  It is very frustrating to 
develop with Java 8 when the stack traces get eaten all the time.

Original comment by ste...@trumpet.io on 5 Apr 2014 at 12:08

[GoogleCodeExporter]

Issue 804 has been merged into this issue.

Original comment by mccu...@gmail.com on 15 Apr 2014 at 10:04

[GoogleCodeExporter]

Ok - so apparently ASM-5.0.2 (or whatever HEAD is at) fixes yet another little 
niggling java8-issue with line-numbers, so as soon as that is released, we can 
try to roll something that is as up-to-date as possible, and see if that 
addresses the last remaining Java8 weirdnesses that come from ASM.

Original comment by christia...@gmail.com on 15 Apr 2014 at 6:34

[GoogleCodeExporter]

ASM-5.0.2 is released. When do you plan to release new version ?

Original comment by srautur...@gmail.com on 20 May 2014 at 4:57

[GoogleCodeExporter]

I recently decided to use Guice with Java 8 on an experimental project.  This 
issue is killing me, I keep getting these "Exception in thread "main" 
com.google.inject.internal.util.$ComputationException: 
java.lang.ArrayIndexOutOfBoundsException: 52264" errors - which provide no 
indication as to what the actual problem is.

Due to this problem, it seems clear to me that Guice is not ready for 
production use with Java 8 :-/

Original comment by ian.clarke on 26 May 2014 at 8:44

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Issue 804 has been merged into this issue.

Original comment by sberlin on 1 Jun 2014 at 7:55

[GoogleCodeExporter]

Just a note that upgrading to 4.0-beta4 seems to have solved the problem.

Original comment by ian.clarke on 2 Jun 2014 at 5:22

[GoogleCodeExporter]

For me, using the no-aop version worked for me. Make sure to exclude 
aopalliance from any maven dependencies you may have. After that Guice reported 
dependency issues correctly. 

Original comment by JWelling...@gmail.com on 2 Jun 2014 at 7:18

[GoogleCodeExporter]

I'm seeing the exception reported in 
https://code.google.com/p/google-guice/issues/detail?id=804 with 4.0-beta4 
still.  I've been unable to figure out yet which changes to our code have 
started causing it.

Original comment by de...@unbounce.com on 15 Jul 2014 at 7:35

[GoogleCodeExporter]

The code.google.com guice project has migrated to GitHub.  This issue site is 
no longer being used.  Please use https://github.com/google/guice/issues/757 
instead.

Original comment by sberlin on 15 Jul 2014 at 7:38

• Changed state: MigratedToGitHub