tikinghu / google-collections

Automatically exported from code.google.com/p/google-collections
0 stars 0 forks source link

Audit System.arraycopy() calls for GWT safety #272

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
arraycopy() doesn't work right in GWT:
http://code.google.com/p/google-web-toolkit/issues/detail?id=3621

We've suppressed some tests because of this bug, but the right thing is to
stop calling arraycopy() when it's not guaranteed to work.

We should also look at all uses of arraycopy() in the library to verify
that they won't trigger this bug.

An easy solution is to create Platform.arraycopy() and substitute a manual
element-by-element copy on the GWT side.  We'd then use
Platform.arraycopy() exclusively.  This has the disadvantage that GWT can
never use its faster nativeArraycopy(), but it's easier than proving that
we're using arraycopy() only when it's safe.

http://www.google.com/codesearch/p?hl=en&sa=N&cd=1&ct=rc#A1edwVHBClQ/user/super/
com/google/gwt/emul/java/lang/System.java&q=System.java%20package:http://google-
web-toolkit\.googlecode\.com

Original issue reported on code.google.com by cpov...@google.com on 23 Oct 2009 at 9:10

GoogleCodeExporter commented 9 years ago

Original comment by cpov...@google.com on 23 Oct 2009 at 9:15

GoogleCodeExporter commented 9 years ago
And how hard would it be to fix GWT?

Original comment by yrfselrahc@gmail.com on 24 Oct 2009 at 1:03

GoogleCodeExporter commented 9 years ago
Well, there's a patch attached to the GWT bug entry already, so we might try
agitating there.

That aside, it would be nice if we didn't ship a library known to trigger bugs 
in the
current version of GWT, but it's not out of the question.

Original comment by cpov...@google.com on 24 Oct 2009 at 4:06

GoogleCodeExporter commented 9 years ago
src/com/google/common/collect/ImmutableSet.java:311
src/com/google/common/collect/ImmutableSet.java:322
src/com/google/common/collect/ImmutableSortedSet.java:240
src/com/google/common/collect/RegularImmutableList.java:70
src/com/google/common/collect/RegularImmutableList.java:80
src/com/google/common/collect/RegularImmutableSortedSet.java:151
src/com/google/common/collect/RegularImmutableSortedSet.java:163
src/com/google/common/collect/ObjectArrays.java:68
src/com/google/common/collect/ObjectArrays.java:69
src/com/google/common/collect/ObjectArrays.java:85
src/com/google/common/collect/ObjectArrays.java:107
src/com/google/common/collect/ImmutableList.java:383
test/com/google/common/collect/SetsTest.java:146
test/com/google/common/collect/SetsTest.java:291
test/com/google/common/collect/ListsTest.java:100
test/com/google/common/collect/ListsTest.java:112
testfw/com/google/common/collect/testing/MinimalCollection.java:88
testfw/com/google/common/collect/testing/AbstractIteratorTester.java:415
testfw/com/google/common/collect/testing/TestMapEntrySetGenerator.java:45

Original comment by kevin...@gmail.com on 2 Nov 2009 at 11:14

GoogleCodeExporter commented 9 years ago
I looked at these, and most of them are explicitly copying from "Object[]" to 
"Object[]" -- does that mean there's no problem?  Or can it still break if 
someone 
passes a SomeInterface[] in as the Object[] to copy?

The ones that might bear a closer look seem to be:

ObjectArrays.concat(T, T[])
ObjectArrays.concat(T[], T)

test framework (not gwtcompatible anyway, right?)

MinimalCollection.toArray()
TestMapEntrySetGenerator? 

Original comment by kevin...@gmail.com on 2 Nov 2009 at 11:22

GoogleCodeExporter commented 9 years ago
It can be a problem if someone passes a SomeInterface[] as the Object[].  Our 
playing
fast and loose with E[], E..., Object[], and Object... (or whichever 
combinations
we've used) makes it hard to know what's happening.

Original comment by cpov...@google.com on 2 Nov 2009 at 11:33

GoogleCodeExporter commented 9 years ago
GWT support is decommissioned for 1.0.

Original comment by kevin...@gmail.com on 6 Nov 2009 at 1:26

GoogleCodeExporter commented 9 years ago
This issue has been moved to the Guava project (keeping the same id number). 
Simply replace 'google-collections' with 'guava-libraries' in your address 
bar and it should take you there.

Original comment by kevinb@google.com on 5 Jan 2010 at 11:09