Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec

[Ulf Zibis]

Am 09.03.2012 09:16, schrieb Sean Chou:
> Hi all,
>
>     AbstractCollection.toArray(T[] ) might return a new array even if the given array has enough 
> room for the returned elements when it is concurrently modified. This behavior violates the spec 
> documented in java.util.Collection .
>     This patch checks the size of returned array and copies the elements to return to the given 
> array if it is large enough.
>
>     The webrev is at :
> http://cr.openjdk.java.net/~zhouyx/7121314/webrev.00/ 
> <http://cr.openjdk.java.net/%7Ezhouyx/7121314/webrev.00/>

More compact and marginally faster:
  182             if (!it.hasNext()) { // fewer elements than expected
  183                 if (a == r) {
  184                     a[i] = null; // null-terminate
  185                 } else if (a.length < i) {
  186                     return Arrays.copyOf(r, i);
  187                 } else {
  188                     System.arraycopy(r, 0, a, 0, Math.min(++i, a.length()); // ensure 
null-termination
  189                 }
  190                 return a;
  191             }

> There is a test case in the previous discussion. It is not included in the webrev, because the 
> testcase is heavily implementation dependent. I will add it if it is requested.
I think, we should have a testcase for all 3 cases: fewer / equal / less elements than expected.
Additionally I think, the correct null-termination should be tested.

>          Thread[] threads = new Thread[2];
>          threads[0] = new Thread(new ToArrayThread());
>          threads[1] = new Thread(new RemoveThread());
Why so complicated?
IMHO better:
         Thread toArrayThread = new Thread(new ToArrayThread());
         Thread removeThread = new Thread(new RemoveThread());

- Ulf