DaveAKing / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

JDK and Guava TypeVariable implementations are no longer compatible under 1.7.0_51-b13 #1635

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
com.google.common.reflect.TypeResolverTest
com.google.common.reflect.TypeTokenResolutionTest
com.google.common.reflect.TypeTokenTest
com.google.common.reflect.TypesTest

java -version

java version "1.7.0_51"
Java(TM) SE Runtime Environment (build 1.7.0_51-b13)
Java HotSpot(TM) 64-Bit Server VM (build 24.51-b03, mixed mode)

please fix it :)

Original issue reported on code.google.com by magician...@gmail.com on 17 Jan 2014 at 1:02

GoogleCodeExporter commented 9 years ago
Can you provide stack traces so we might have a better idea of what is going 
wrong in your code?  Our tests pass internally.

Original comment by wasserman.louis on 17 Jan 2014 at 3:02

GoogleCodeExporter commented 9 years ago
The testcase already exists in the project , not my own .   Here is the  stack 
traces :
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.google.common.reflect.AbstractInvocationHandlerTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.192 sec
Running com.google.common.reflect.ClassPathTest

Jan 17, 2014 4:26:46 PM com.google.common.reflect.ClassPath$Scanner 
getClassPathFromManifest
WARNING: Invalid Class-Path entry: an_invalid^path
Tests run: 35, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.218 sec
Running com.google.common.reflect.ElementTest
Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.019 sec
Running com.google.common.reflect.ImmutableTypeToInstanceMapTest
Tests run: 611, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.358 sec
Running com.google.common.reflect.InvokableTest
Tests run: 52, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.11 sec
Running com.google.common.reflect.MutableTypeToInstanceMapTest
Tests run: 685, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.705 sec
Running com.google.common.reflect.ParameterTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.004 sec
Running com.google.common.reflect.ReflectionTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.009 sec
Running com.google.common.reflect.TypeParameterTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 sec
Running com.google.common.reflect.TypeResolverTest
Tests run: 23, Failures: 5, Errors: 0, Skipped: 0, Time elapsed: 0.018 sec <<< 
FAILURE!
Running com.google.common.reflect.TypesTest
Tests run: 26, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.029 sec <<< 
FAILURE!
Running com.google.common.reflect.TypeTokenResolutionTest
Tests run: 35, Failures: 3, Errors: 0, Skipped: 0, Time elapsed: 0.03 sec <<< 
FAILURE!
Running com.google.common.reflect.TypeTokenTest
Tests run: 145, Failures: 5, Errors: 0, Skipped: 0, Time elapsed: 0.168 sec <<< 
FAILURE!
Running com.google.common.reflect.TypeVisitorTest
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.003 sec

Results :

Failed tests: 
  testWhere_noMapping(com.google.common.reflect.TypeResolverTest): expected:<T> but was:<T>
  testWhere_typeVariableSelfMapping(com.google.common.reflect.TypeResolverTest): expected:<T> but was:<T>
  testWhere_parameterizedSelfMapping(com.google.common.reflect.TypeResolverTest): expected:<java.util.List<T>> but was:<java.util.List<T>>
  testWhere_genericArraySelfMapping(com.google.common.reflect.TypeResolverTest): expected:<T[]> but was:<T[]>
  testWhere_wildcardSelfMapping(com.google.common.reflect.TypeResolverTest): expected:<? extends T> but was:<? extends T>
  testNewTypeVariable(com.google.common.reflect.TypesTest): T [group 1, item 1] must be Object#equals to T [group 1, item 2]
  testInnerClassWithParameterizedOwner(com.google.common.reflect.TypeTokenResolutionTest): expected:<com.google.common.reflect.TypeTokenResolutionTest.com.google.common.reflect.TypeTokenResolutionTest$ParameterizedOuter<T>.Inner> but was:<com.google.common.reflect.TypeTokenResolutionTest.com.google.common.reflect.TypeTokenResolutionTest$ParameterizedOuter<T>.com.google.common.reflect.TypeTokenResolutionTest$ParameterizedOuter$Inner<>>
  testConextIsParameterizedType(com.google.common.reflect.TypeTokenResolutionTest): expected:<K> but was:<K>
  testGenericArrayType(com.google.common.reflect.TypeTokenResolutionTest): expected:<T> but was:<T>
  testGetSupertype_withTypeVariable(com.google.common.reflect.TypeTokenTest): expected:<java.lang.Iterable<java.util.List<T>>> but was:<java.lang.Iterable<java.util.List<T>>>
  testGetSupertype_fromRawClass(com.google.common.reflect.TypeTokenTest): expected:<java.lang.Iterable<E>> but was:<java.lang.Iterable<E>>
  testGetSupertype_fullyGenericType(com.google.common.reflect.TypeTokenTest): expected:<java.util.Map<K, java.util.List<V>>> but was:<java.util.Map<K, java.util.List<V>>>
  testGetSupertype_partiallySpecializedType(com.google.common.reflect.TypeTokenTest): expected:<java.util.Map<java.lang.String, java.util.List<V>>> but was:<java.util.Map<java.lang.String, java.util.List<V>>>
  testWhere(com.google.common.reflect.TypeTokenTest): expected:<java.util.Map<java.lang.String, java.lang.Integer>> but was:<java.util.Map<java.lang.String, V>>

Tests run: 1652, Failures: 14, Errors: 0, Skipped: 0

Original comment by magician...@gmail.com on 17 Jan 2014 at 8:29

GoogleCodeExporter commented 9 years ago
Here is the command :
mvn test -Dtest.include=**/com/google/common/reflect/*Test.java

Original comment by magician...@gmail.com on 17 Jan 2014 at 8:33

GoogleCodeExporter commented 9 years ago
Things are OK at 1.7.0_45 but definitely broken at 1.7.0_51-b13

I wonder if the problem is in the JDK itself. Maybe they broke their equals() 
methods?

Here are some stack traces:

Running com.google.common.reflect.TypeResolverTest
Tests run: 23, Failures: 5, Errors: 0, Skipped: 0, Time elapsed: 0.02 sec <<< 
FAILURE!
testWhere_noMapping(com.google.common.reflect.TypeResolverTest)  Time elapsed: 
0.002 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<T> but was:<T>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeResolverTest.testWhere_noMapping(TypeResolverTest.java:35)

testWhere_typeVariableSelfMapping(com.google.common.reflect.TypeResolverTest)  
Time elapsed: 0.001 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<T> but was:<T>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeResolverTest.testWhere_typeVariableSelfMapping(TypeResolverTest.java:53)

testWhere_parameterizedSelfMapping(com.google.common.reflect.TypeResolverTest)  
Time elapsed: 0.002 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<java.util.List<T>> but 
was:<java.util.List<T>>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeResolverTest.testWhere_parameterizedSelfMapping(TypeResolverTest.java:59)

testWhere_genericArraySelfMapping(com.google.common.reflect.TypeResolverTest)  
Time elapsed: 0 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<T[]> but was:<T[]>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeResolverTest.testWhere_genericArraySelfMapping(TypeResolverTest.java:65)

testWhere_wildcardSelfMapping(com.google.common.reflect.TypeResolverTest)  Time 
elapsed: 0.002 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<? extends T> but was:<? extends 
T>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeResolverTest.testWhere_wildcardSelfMapping(TypeResolverTest.java:77)

Tests run: 26, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.066 sec <<< 
FAILURE!
testNewTypeVariable(com.google.common.reflect.TypesTest)  Time elapsed: 0.001 
sec  <<< FAILURE!
junit.framework.AssertionFailedError: T [group 1, item 1] must be Object#equals 
to T [group 1, item 2]
  at com.google.common.testing.RelationshipTester.assertWithTemplate(RelationshipTester.java:123)
  at com.google.common.testing.RelationshipTester.assertRelated(RelationshipTester.java:103)
  at com.google.common.testing.RelationshipTester.test(RelationshipTester.java:79)
  at com.google.common.testing.EqualsTester.testEquals(EqualsTester.java:112)
  at com.google.common.reflect.TypesTest.testNewTypeVariable(TypesTest.java:314)

Running com.google.common.reflect.TypeTokenResolutionTest
Tests run: 35, Failures: 3, Errors: 0, Skipped: 0, Time elapsed: 0.015 sec <<< 
FAILURE!
testInnerClassWithParameterizedOwner(com.google.common.reflect.TypeTokenResoluti
onTest)  Time elapsed: 0.001 sec  <<< FAILURE!
junit.framework.AssertionFailedError: 
expected:<com.google.common.reflect.TypeTokenResolutionTest.com.google.common.re
flect.TypeTokenResolutionTest$ParameterizedOuter<T>.Inner> but 
was:<com.google.common.reflect.TypeTokenResolutionTest.com.google.common.reflect
.TypeTokenResolutionTest$ParameterizedOuter<T>.com.google.common.reflect.TypeTok
enResolutionTest$ParameterizedOuter$Inner<>>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeTokenResolutionTest.testInnerClassWithParameterizedOwner(TypeTokenResolutionTest.java:231)

testConextIsParameterizedType(com.google.common.reflect.TypeTokenResolutionTest)
  Time elapsed: 0 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<K> but was:<K>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeTokenResolutionTest.testConextIsParameterizedType(TypeTokenResolutionTest.java:269)

testGenericArrayType(com.google.common.reflect.TypeTokenResolutionTest)  Time 
elapsed: 0 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<T> but was:<T>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeTokenResolutionTest.testGenericArrayType(TypeTokenResolutionTest.java:280)

Running com.google.common.reflect.TypeTokenTest
Tests run: 145, Failures: 5, Errors: 0, Skipped: 0, Time elapsed: 0.096 sec <<< 
FAILURE!
testGetSupertype_withTypeVariable(com.google.common.reflect.TypeTokenTest)  
Time elapsed: 0 sec  <<< FAILURE!
junit.framework.AssertionFailedError: 
expected:<java.lang.Iterable<java.util.List<T>>> but 
was:<java.lang.Iterable<java.util.List<T>>>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeTokenTest.testGetSupertype_withTypeVariable(TypeTokenTest.java:1003)

testGetSupertype_fromRawClass(com.google.common.reflect.TypeTokenTest)  Time 
elapsed: 0 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<java.lang.Iterable<E>> but 
was:<java.lang.Iterable<E>>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeTokenTest.testGetSupertype_fromRawClass(TypeTokenTest.java:1050)

testGetSupertype_fullyGenericType(com.google.common.reflect.TypeTokenTest)  
Time elapsed: 0 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<java.util.Map<K, 
java.util.List<V>>> but was:<java.util.Map<K, java.util.List<V>>>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeTokenTest.testGetSupertype_fullyGenericType(TypeTokenTest.java:1073)

testGetSupertype_partiallySpecializedType(com.google.common.reflect.TypeTokenTes
t)  Time elapsed: 0 sec  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<java.util.Map<java.lang.String, 
java.util.List<V>>> but was:<java.util.Map<java.lang.String, java.util.List<V>>>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeTokenTest.testGetSupertype_partiallySpecializedType(TypeTokenTest.java:1087)

testWhere(com.google.common.reflect.TypeTokenTest)  Time elapsed: 0.001 sec  
<<< FAILURE!
junit.framework.AssertionFailedError: expected:<java.util.Map<java.lang.String, 
java.lang.Integer>> but was:<java.util.Map<java.lang.String, V>>
  at junit.framework.Assert.fail(Assert.java:47)
  at junit.framework.Assert.failNotEquals(Assert.java:283)
  at junit.framework.Assert.assertEquals(Assert.java:64)
  at junit.framework.Assert.assertEquals(Assert.java:71)
  at com.google.common.reflect.TypeTokenTest.testWhere(TypeTokenTest.java:1175)

Original comment by cpov...@google.com on 17 Jan 2014 at 6:08

GoogleCodeExporter commented 9 years ago
Yeah, I was just testing it and got the same thing. It does seem likely that 
the problem is in the JDK.

Original comment by cgdecker@google.com on 17 Jan 2014 at 6:12

GoogleCodeExporter commented 9 years ago
I looked a little more at 
TypeTokenResolutionTest#testConextIsParameterizedType, which seemed like one of 
the simpler tests.

Under both JDK versions, keyType is a 
sun.reflect.generics.reflectiveObjects.TypeVariableImpl, and 
TypeToken.of(keyType).resolveType(keyType).getType() is a 
com.google.common.reflect.Types$TypeVariableImpl.

The tests pass if I reverse the order of the arguments to assertEquals. This 
further supports the theory that the JDK equals() method is broken. More 
explicitly, this test passes, but the reverse does not:

  assertTrue(TypeToken.of(keyType).resolveType(keyType).getType().equals(keyType));

However, I also diffed the output of javap -v for 
sun.reflect.generics.reflectiveObjects.TypeVariableImpl between the JDKs, and 
it matches exactly. Of course, the problem may lie in a method called by 
TypeVariableImpl.

Original comment by cpov...@google.com on 17 Jan 2014 at 6:20

GoogleCodeExporter commented 9 years ago
Err, no, I think I got that last part wrong. I'm running javap -cp 
jre/lib/rt.jar, but I'm probably getting the version from the bootstrap 
classpath. I'll redo it.

Original comment by cpov...@google.com on 17 Jan 2014 at 6:23

GoogleCodeExporter commented 9 years ago
Sure enough, the new TypeVariableImpl is different. It does an instanceof check 
*and* a getClass() == TypeVariableImpl.class check. Odd.

Original comment by cpov...@google.com on 17 Jan 2014 at 6:29

GoogleCodeExporter commented 9 years ago
I tried to find the location of the JDK's TypeVariableImpl. However, the copy I 
see in Mercurial looks like the 1.7.0_45 version that works fine with our tests.

http://hg.openjdk.java.net/jdk7/tl/jdk/file/cfd7602f5c52/src/share/classes/sun/r
eflect/generics/reflectiveObjects/TypeVariableImpl.java

Maybe I need to find a branch or an Oracle-specific repository. I'm not 
familiar with the JDK release process.

Original comment by cpov...@google.com on 17 Jan 2014 at 6:36

GoogleCodeExporter commented 9 years ago
In fact, that whole repository seems to have not been touched in >2 years.

Original comment by cpov...@google.com on 17 Jan 2014 at 6:38

GoogleCodeExporter commented 9 years ago
Well, I see the change in a JDK8 repository:

http://hg.openjdk.java.net/jdk8/tl/jdk/diff/b90047350153/src/share/classes/sun/r
eflect/generics/reflectiveObjects/TypeVariableImpl.java

The change is listed under "Security fixes" in some release notes:

"S8023301: Enhance generic classes"

http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2014-January/025800.html

Original comment by cpov...@google.com on 17 Jan 2014 at 6:43

GoogleCodeExporter commented 9 years ago
In hg, you need to look in jdk7u/jdk7u, not jdk7/jdk7. jdk7u is where update 
work goes.

Original comment by cgdecker@google.com on 17 Jan 2014 at 6:59

GoogleCodeExporter commented 9 years ago
Here's the change that introduced the "o.getClass() == TypeVariableImpl.class" 
check: 
http://hg.openjdk.java.net/jdk7u/jdk7u/jdk/diff/8e542d28b8ec/src/share/classes/s
un/reflect/generics/reflectiveObjects/TypeVariableImpl.java

Original comment by cgdecker@google.com on 17 Jan 2014 at 7:02

GoogleCodeExporter commented 9 years ago
Issue 1637 has been merged into this issue.

Original comment by cgdecker@google.com on 17 Jan 2014 at 7:13

GoogleCodeExporter commented 9 years ago
Issue 1637 has been merged into this issue.

Original comment by cpov...@google.com on 17 Jan 2014 at 7:14

GoogleCodeExporter commented 9 years ago

Original comment by cpov...@google.com on 17 Jan 2014 at 7:16

GoogleCodeExporter commented 9 years ago
I appears that this was closed resolved in their JIRA tracker:

   https://bugs.openjdk.java.net/browse/JDK-8031984

Original comment by john....@socrata.com on 20 Jan 2014 at 4:55

GoogleCodeExporter commented 9 years ago
To my eyes the description in https://bugs.openjdk.java.net/browse/JDK-8031984 
doesn't make a very compelling case that this is a bug in the JDK - would it be 
possible to make a more compelling case there, for example by having a test 
case that doesn't use internal sun.reflect classes?

Original comment by frank...@gmail.com on 20 Jan 2014 at 5:47

GoogleCodeExporter commented 9 years ago
> doesn't make a very compelling case that this is a bug

Trying to take a quick step back here: *is* this actually a bug? I mean, is 
there any kind of documented contract that says that 'equals' on TypeVariables 
should allow for other implementing classes?

The Javadoc for 7 [1] *does* say:

"However, all instances representing a type variable must be equal() to each 
other. As a consequence, users of type variables must not rely on the identity 
of instances of classes implementing this interface."

Could one use this as justification for calling this a bug? I guess Oracle 
might argue that users shouldn't be creating "instances representing a type 
variable" on their own in the first place?

[1] http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/TypeVariable.html

Original comment by sharedo...@gmail.com on 20 Jan 2014 at 6:07

GoogleCodeExporter commented 9 years ago
This is a bug. I now have a production failure which has been associated to 
this, created due to automatic CentOS updates.  I recommend that a fix is 
created and released ASAP, and we worry about the semantics later.

Original comment by nwil...@gmail.com on 20 Jan 2014 at 8:16

GoogleCodeExporter commented 9 years ago
> I recommend that a fix is created and released ASAP, and we worry about the 
semantics later.

Fully agree with that (this has broken a lot of stuff on our side, too). I 
should have made it clearer that I wasn't talking about *this* ticket, but 
about Oracle's response in closing the issue raised against the JDK.

Original comment by sharedo...@gmail.com on 20 Jan 2014 at 8:21

GoogleCodeExporter commented 9 years ago
Some of these broken tests could be worked around by checking whether the 
component types have changed through transformation, or else use the original 
TypeVariable instance.

if (bounds == originalBounds) {
  return originalType;
}
return Types.newTypeVariable(...);

This doesn't address the case where the type variable bound went through 
transformation. For that, options I can think of:

1. Depend on sun.reflect classes and worry about incompatibility issues later.
2. Just don't resolve type variable bounds at all.

Neither feels good enough.

Original comment by be...@google.com on 21 Jan 2014 at 12:46

GoogleCodeExporter commented 9 years ago
Note that this breaks downstream libraries that depend on guava as well, so 
this isn't just an issue of test code in Guava. Here's just one example: 
https://issues.apache.org/jira/browse/JCLOUDS-427

Given the number of proprietary and open source projects that depend on Guava, 
and the fact that many systems will have silent, automatic upgrades of the JDK, 
I suspect this will impact a large number of users soon if it hasn't already.

In fact, due to dependency conflicts, we are not able to upgrade to Guava 16 
yet so a backport to Guava 15.x will be hugely appreciated!

Original comment by diwakerg...@gmail.com on 21 Jan 2014 at 6:50

GoogleCodeExporter commented 9 years ago
Suppose that we want to create an instance of the JDK's TypeVariableImpl for 
List<String>. Might we be able to use something like ASM/cglib to generate a 
custom type, HasTypeParameterForListString<T extends List<String>>? Then we 
could ask standard Java reflection for HasTypeParameterForListString's type 
parameter's bound, at which point we'd get a JDK TypeVariableImpl back.

Original comment by cpov...@google.com on 23 Jan 2014 at 11:54

GoogleCodeExporter commented 9 years ago
But that TypeVariableImpl wouldn't have the right GenericDeclaration, so it 
still wouldn't compare equal.

Original comment by tavianator@gmail.com on 24 Jan 2014 at 12:02

GoogleCodeExporter commented 9 years ago
Oh, I see. My attempted solution doesn't account for retrieving type variables 
at all. The TypeVariableImpl that I'm retrieving is for <T extends 
List<String>>. But I don't care about that; I care about List<String>. I can 
get it from getBounds(), and that part works fine (I suspect). But what if I 
want List<E>, where E is declared in some random class or method? (After all, 
it's the TypeVariable for E that's the problem here.) I doubt there's a general 
way to declare a type that refers to such a thing. But I'd love for someone to 
prove me wrong....

Original comment by cpov...@google.com on 24 Jan 2014 at 12:20

GoogleCodeExporter commented 9 years ago
We may try the terrible thing -- manually creating sun.reflect.generics types. 
But we would do it only in the presence of the new, problematic 
TypeVariableImpl and only under the guard of reflection. I'm running some tests 
internally now to see how well an early attempt works.

Original comment by cpov...@google.com on 24 Jan 2014 at 4:51

GoogleCodeExporter commented 9 years ago
This defect is receiving a great deal of attention from the wider community:
http://www.reddit.com/r/java/comments/1vxvr8/java_security_patch_breaks_guava_li
brary/

I suggest that a fix is identified as soon as possible to resolve this defect 
and minimise further impact on Guava users. I've also opened Issue #1644 to 
work on removing all dependency on unsupported API classes.

Original comment by nick.wil...@jds.net.au on 24 Jan 2014 at 11:15

GoogleCodeExporter commented 9 years ago
Chris, "under the guard of reflection" seems to imply some sort of fallback, 
but to what?

Original comment by kevinb@google.com on 24 Jan 2014 at 4:32

GoogleCodeExporter commented 9 years ago
To the current behavior. Jeremy points out that, as far as we know[*], there 
are two kinds of reflection-enabled Java environments in the world:

1. recent ones written by sun, for which we hope that the proposed sun.reflect 
hack will work
2. other environments, for which our TypeVariable implementation has already 
been working well enough

[*] Yes, that's not very encouraging.

Original comment by cpov...@google.com on 24 Jan 2014 at 5:14

GoogleCodeExporter commented 9 years ago
RE: #22, if the bounds go through transformation, does it even make sense to 
have the new TypeVariable compare equal to the old one?

Original comment by tavianator@gmail.com on 24 Jan 2014 at 5:37

GoogleCodeExporter commented 9 years ago
Yeah. That's a good question. I'm having a hard time imagining why it would 
matter.

So I think it might be a good solution.

Need to give it a bit more thought. And probably start experimenting it.

Thanks!

Original comment by be...@google.com on 24 Jan 2014 at 5:50

GoogleCodeExporter commented 9 years ago
> Oracle might argue that users shouldn't be creating "instances representing a 
type variable" on their own in the first place?

There seems to be no explicit prohibition; the closest I can find is this note 
earlier in the Javadoc:

> A type variable is created the first time it is needed by a reflective 
method, as specified in this package.

which vaguely implies that implementations outside the JRE are not supported.

Since Java provides no way of restricting what code may implement an interface 
(other than using annotations and annotation processors not defined in the 
Platform), API documentation should clearly state how an interface may be used. 
In this case the designers of the reflection API apparently failed to consider 
the possibility.

Original comment by jgl...@cloudbees.com on 24 Jan 2014 at 9:46

GoogleCodeExporter commented 9 years ago
For what it's worth, we (jclouds) have been able to workaround the failures 
this was causing by switching to other Guava reflection methods. See 
https://issues.apache.org/jira/browse/JCLOUDS-427 for details.

Obviously, this is not a solution to the ticket, but might help other users 
that call similar Guava functions.

Original comment by sharedo...@gmail.com on 27 Jan 2014 at 7:18

GoogleCodeExporter commented 9 years ago
Does someone know of a simple test to check if an application would be affected 
by this bug - also considering the possibility that the app is not using the 
'broken' methods directly but only indirectly via a 'downstream' library?

Original comment by cku...@gmail.com on 29 Jan 2014 at 2:46

GoogleCodeExporter commented 9 years ago
> Does someone know of a simple test to check if an application would be 
affected by this bug

Would this be a test you would want to include in Guava test suite, or 
something a Guava user (directly or via downstream libs) could run against 
his/her application to test this?

I presume that, in the latter case, the easiest thing would probably be to run 
the app against 7u51 to see what happens..?

Original comment by sharedo...@gmail.com on 29 Jan 2014 at 7:54

GoogleCodeExporter commented 9 years ago
The latter one; Something a Guava user (directly or via downstream libs) could 
run against his/her application.
I fear there's no way to do that easily.

It's a bigger 'enterprise' system consisting of many services/apps of whom many 
directly or indirectly depend on Guava.
I guess we'll just have to run our test suite against 7u51 and hope that the 
test coverage is good enough to catch any issues.

Original comment by cku...@gmail.com on 29 Jan 2014 at 9:43

GoogleCodeExporter commented 9 years ago
> Something a Guava user (directly or via downstream libs) could run against 
his/her 
> application.

Do you have some kind of static checker in mind here that could see if you 
*could* run into this problem in one of your code paths? Something like a 
"vulnerability scanner" that a user might run *before* deciding whether or not 
to move to 7u51?

I'm guessing the challenge here is that one would obviously want to flag only 
invocations of the affected methods (directly in app code or via a downstream 
lib) that will actually be called when the app is running in real life?

I.e. rather than simply flagging up every invocation of an affected method 
*anywhere* in the app or downstream libs, even if they will never be invoked.

Original comment by sharedo...@gmail.com on 30 Jan 2014 at 12:20

GoogleCodeExporter commented 9 years ago
Seems like the time spent implementing such a static checker would greatly 
exceed the time needed to just work around the JRE behavior in Guava and 
release a Guava update. (Ideally with emergency patch updates to 11.x and 
subsequent major releases, to allow people stuck for compatibility reasons with 
older Guava versions to keep running.) Of course there is probably already some 
tool out there which traverses the static call tree of an app (starting from 
bytecode you wrote) looking for references to a given class or method.

Original comment by jgl...@cloudbees.com on 30 Jan 2014 at 2:02

GoogleCodeExporter commented 9 years ago
The fix has been submitted:

http://code.google.com/p/guava-libraries/source/detail?r=5f913f5e6411698a4e50a05
93ea4db4632e2c912

We'll put out a patch release for Guava 16 that includes this fix (and a 
forthcoming revision to its documentation).

Original comment by cpov...@google.com on 3 Feb 2014 at 9:07

GoogleCodeExporter commented 9 years ago
Fantastic! Can you also please backport this fix to Guava 15?

Original comment by nwil...@gmail.com on 3 Feb 2014 at 9:12

GoogleCodeExporter commented 9 years ago
> The fix has been submitted:

Wow, great! Thanks!

Original comment by sharedo...@gmail.com on 3 Feb 2014 at 9:14

GoogleCodeExporter commented 9 years ago
@nwilton: Are you unable to switch to Guava 16 for some reason? Just wondering.

Original comment by cgdecker@google.com on 3 Feb 2014 at 9:15

GoogleCodeExporter commented 9 years ago
16.0.1 has been released with this fix: 
http://search.maven.org/#artifactdetails%7Ccom.google.guava%7Cguava%7C16.0.1%7Cb
undle

Original comment by cgdecker@google.com on 4 Feb 2014 at 6:55

GoogleCodeExporter commented 9 years ago
Any plans for 15.0.1 with the same fix?  jclouds upgraded to 16.0.1 in its 
1.8.x development branch but prefers not to upgrade Guava a major version in 
its 1.7.x stable branch due to various Guava deprecations and API changes.

Original comment by g...@maginatics.com on 5 Feb 2014 at 9:04

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:10

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:07