Hi Charles,
Verified. Thanks.
On Tue, Apr 10, 2012 at 2:03 PM, Charles Lee <litt...@linux.vnet.ibm.com>wrote:
> Hi Sean,
>
> The the patch has been committed @
>
> Changeset: e06ea0dd9207
> Author: littlee
> Date: 2012-04-10 10:17 +0800
> URL:http://hg.openjdk.java.**net/jdk8/tl/jdk/rev/**e06ea0dd9207<http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e06ea0dd9207>
>
>
> 7121314: Behavior mismatch between AbstractCollection.toArray(T[] ) and
> its spec
> Reviewed-by: dholmes, mduigou
> Contributed-by: Sean Zhou<zho...@linux.vnet.ibm.com**>, Ulf Zibis<
> ulf.zi...@gmx.de>, David Holmes<david.hol...@oracle.com**>
>
> ! src/share/classes/java/util/**AbstractCollection.java
> + test/java/util/**AbstractCollection/**ToArrayTest.java
>
> Please verified it.
>
> Thank you all for contributing and reviewing this change. Thank you Mike
> and Joe for helping my committing this patch.
>
>
> On 04/01/2012 05:24 PM, Sean Chou wrote:
>
>> Hi David,
>>
>> I made the new webrev with your modified testcase:
>> http://cr.openjdk.java.net/~**zhouyx/7121314/webrev.04/<http://cr.openjdk.java.net/~zhouyx/7121314/webrev.04/>
>> .
>>
>>
>> The link to the bug:
>>
>> http://bugs.sun.com/**bugdatabase/view_bug.do?bug_**id=7121314<http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7121314>
>> The link to the start of the thread:
>>
>> http://mail.openjdk.java.net/**pipermail/core-libs-dev/2012-**
>> March/009512.html<http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-March/009512.html>
>>
>>
>> On Sun, Apr 1, 2012 at 11:25 AM, David Holmes<david.hol...@oracle.com**
>> >wrote:
>>
>> Simplified testcase below.
>>>
>>> Let's finalise this please.
>>>
>>> Thanks,
>>> David
>>>
>>>
>>> On 1/04/2012 1:08 PM, Sean Chou wrote:
>>>
>>> Hi Ulf,
>>>>
>>>> This is a regression testcase, there is no performance issue or
>>>> future refactoring.
>>>> Please wait for David's comments.
>>>>
>>>> On Sun, Apr 1, 2012 at 7:22 AM, Ulf Zibis<ulf.zi...@gmx.de
>>>> <mailto:ulf.zi...@gmx.de>> wrote:
>>>>
>>>> Hi Sean,
>>>>
>>>> thanks for your effort.
>>>>
>>>> Am 31.03.2012 11:43, schrieb Sean Chou:
>>>>
>>>> Hi David and Ulf,
>>>>
>>>> The new webrev is at:
>>>>
>>>> http://cr.openjdk.java.net/~__****zhouyx/7121314/webrev.03/<http://cr.openjdk.java.net/~__**zhouyx/7121314/webrev.03/>
>>>> <ht**tp://cr.openjdk.java.net/~__**zhouyx/7121314/webrev.03/<http://cr.openjdk.java.net/~__zhouyx/7121314/webrev.03/>
>>>> >
>>>>
>>>> <http://cr.openjdk.java.net/~****zhouyx/7121314/webrev.03/<http://cr.openjdk.java.net/~**zhouyx/7121314/webrev.03/>
>>>> <htt**p://cr.openjdk.java.net/~**zhouyx/7121314/webrev.03/<http://cr.openjdk.java.net/~zhouyx/7121314/webrev.03/>
>>>> >
>>>> <http://cr.openjdk.java.net/%_****_7Ezhouyx/7121314/webrev.03/
>>>>
>>>> <http://cr.openjdk.java.net/%****7Ezhouyx/7121314/webrev.03/<h**
>>>> ttp://cr.openjdk.java.net/%**7Ezhouyx/7121314/webrev.03/<http://cr.openjdk.java.net/%7Ezhouyx/7121314/webrev.03/>
>>>> >>>
>>>>
>>>> .
>>>>
>>>>
>>>> About the fix, I remained the previous one.
>>>> About the testcase, I merged the 3 files into one.
>>>> During merging, there are 2 modifications:
>>>> 1. I added static modifier to the 3 classes, which are enclosed
>>>> by class ToArrayTest;
>>>>
>>>> You do not need the indirection via main()...run()...test() if you
>>>> have all in 1 file. This was only necessary to feature a general
>>>> usability of InfraStructure. You can go back to David's 1 + 1 nested
>>>> class approach replacing TConcurrentHashMapX by TestCollection and
>>>> maybe rename realMain() to test().
>>>> Additionally, loading 4 classes for 1 test would have some
>>>> performance impact on the test run, which could be avoided.
>>>>
>>>>
>>>> 2. I removed field TestCollection.fixedSize, which is never read
>>>> after Ulf fixed the bug in testcase.
>>>>
>>>> This field would serve to "reset" the TestCollection to fixed
>>>> default size without the need of new instantiation for later
>>>> refactoring or testcase addition.
>>>>
>>>> As just discussed before, the doc for setSizeSequence() could be
>>>> little more specific:
>>>> 71 /*
>>>> 72 * Sets the values that size() will return on each use.
>>>> The first
>>>> 73 * call to size will return sizes[0], then sizes[1]
>>>> etc. This
>>>> 74 * allows us to emulate a concurrent change to the
>>>> contents of
>>>> 75 * the collection without having to perform concurrent
>>>> changes.
>>>> 76 * If sizes[n+1] contains a larger value than on last
>>>> n-th invocation,
>>>> 77 * the collection will appear to have shrunk when
>>>> iterated; if a
>>>> 78 * smaller value then the collection will appear to
>>>> have grown.
>>>> 79 * When the last element of sizes is reached, the
>>>> collection will
>>>> 80 * appear size-fixed.
>>>> 81 */
>>>>
>>>>
>>>> The link to the bug:
>>>> http://bugs.sun.com/__****bugdatabase/view_bug.do?bug___**
>>>> **id=7121314<http://bugs.sun.com/__**bugdatabase/view_bug.do?bug___**id=7121314>
>>>> <http://bugs.sun.**com/__bugdatabase/view_bug.do?**bug___id=7121314<http://bugs.sun.com/__bugdatabase/view_bug.do?bug___id=7121314>
>>>> >
>>>>
>>>> <http://bugs.sun.com/****bugdatabase/view_bug.do?bug_****
>>>> id=7121314<http://bugs.sun.com/**bugdatabase/view_bug.do?bug_**id=7121314>
>>>> <http://bugs.sun.**com/bugdatabase/view_bug.do?**bug_id=7121314<http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7121314>
>>>> >
>>>>
>>>> The link to the start of the thread:
>>>> http://mail.openjdk.java.net/_****_pipermail/core-libs-dev/**
>>>> 2012-**<http://mail.openjdk.java.net/_**_pipermail/core-libs-dev/2012-**>
>>>> __March/009512.html<http://**mail.openjdk.java.net/__**
>>>> pipermail/core-libs-dev/2012-_**_March/009512.html<http://mail.openjdk.java.net/__pipermail/core-libs-dev/2012-__March/009512.html>
>>>> >
>>>>
>>>> <http://mail.openjdk.java.net/****pipermail/core-libs-dev/**
>>>> 2012-**<http://mail.openjdk.java.net/**pipermail/core-libs-dev/2012-**>
>>>> March/009512.html<http://mail.**openjdk.java.net/pipermail/**
>>>> core-libs-dev/2012-March/**009512.html<http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-March/009512.html>
>>>> >
>>>>
>>>> Good idea, to repeat these links.
>>>>
>>>> -Ulf
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Best Regards,
>>>> Sean Chou
>>>>
>>>>
>>>> /*
>>> * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
>>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>> *
>>> * This code is free software; you can redistribute it and/or modify it
>>> * under the terms of the GNU General Public License version 2 only, as
>>> * published by the Free Software Foundation.
>>> *
>>> * This code is distributed in the hope that it will be useful, but
>>> WITHOUT
>>> * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>> * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
>>> * version 2 for more details (a copy is included in the LICENSE file
>>> that
>>> * accompanied this code).
>>> *
>>> * You should have received a copy of the GNU General Public License
>>> version
>>> * 2 along with this work; if not, write to the Free Software Foundation,
>>> * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
>>> *
>>> * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065
>>> USA
>>> * or visit www.oracle.com if you need additional information or have
>>> any
>>> * questions.
>>> */
>>>
>>> /*
>>> * @test
>>> * @bug 7121314
>>> * @summary AbstractCollection.toArray(T[]****) doesn't return the given
>>>
>>> array
>>> * in concurrent modification.
>>> * @author Ulf Zibis, David Holmes
>>> */
>>>
>>> import java.util.AbstractCollection;
>>> import java.util.Arrays;
>>> import java.util.Iterator;
>>>
>>> public class ToArrayTest {
>>>
>>> static class TestCollection<E> extends AbstractCollection<E> {
>>>
>>> private final E[] elements;
>>>
>>>
>>> private int[] sizes;
>>> private int nextSize;
>>>
>>> public TestCollection(E[] elements) {
>>> this.elements = elements;
>>> setSizeSequence(new int[]{elements.length});
>>> }
>>>
>>> /*
>>> * Sets the values that size() will return on each use. The next
>>>
>>> * call to size will return sizes[0], then sizes[1] etc. This
>>> * allows us to emulate a concurrent change to the contents of
>>> * the collection without having to perform concurrent changes.
>>> * If sizes[n+1] contains a larger value, the collection will
>>> appear to
>>> * have shrunk when iterated; if a smaller value then the
>>> * collection will appear to have grown when iterated
>>>
>>> */
>>> void setSizeSequence(int... sizes) {
>>> this.sizes = sizes;
>>> nextSize = 0;
>>> }
>>>
>>> /* can change collection's size after each invocation */
>>>
>>> @Override
>>> public int size() {
>>> return sizes[nextSize == sizes.length-1 ? nextSize :
>>> nextSize++];
>>> }
>>>
>>> @Override
>>> public Iterator<E> iterator() {
>>> return new Iterator<E>() {
>>>
>>> int pos = 0;
>>> public boolean hasNext() {
>>> return pos< sizes[nextSize];
>>> }
>>> public E next() {
>>> return elements[pos++];
>>> }
>>> public void remove() {
>>> throw new UnsupportedOperationException(****"Not
>>>
>>> supported yet.");
>>> }
>>> };
>>> }
>>> }
>>>
>>> static final Object[] OBJECTS = { new Object(), new Object(), new
>>> Object() };
>>> static final TestCollection<?> CANDIDATE = new
>>> TestCollection<Object>(**
>>>
>>> OBJECTS);
>>> static final int CAP = OBJECTS.length; // capacity of the CANDIDATE
>>> static final int LAST = CAP - 1; // last possible array index
>>> Object[] a;
>>> Object[] res;
>>>
>>> int last() { return a.length - 1; }
>>>
>>> protected void test() throws Throwable {
>>> // Check array type conversion
>>> res = new TestCollection<>(new Object[]{"1", "2"}).toArray(new
>>> String[0]);
>>> check(res instanceof String[]);
>>> check(res.length == 2);
>>> check(res[1] == "2");
>>>
>>> // Check incompatible type of target array
>>> try {
>>> res = CANDIDATE.toArray(new String[CAP]);
>>> check(false);
>>> } catch (Throwable t) {
>>> check(t instanceof ArrayStoreException);
>>> }
>>>
>>> // Check more elements than a.length
>>> a = new Object[CAP-1]; // appears too small
>>> res = CANDIDATE.toArray(a);
>>> check(res != a);
>>> check(res[LAST] != null);
>>>
>>> // Check equal elements as a.length
>>> a = new Object[CAP]; // appears to match
>>> res = CANDIDATE.toArray(a);
>>> check(res == a);
>>> check(res[last()] != null);
>>>
>>> // Check equal elements as a.length
>>> a = new Object[CAP+1]; // appears too big
>>> res = CANDIDATE.toArray(a);
>>> check(res == a);
>>> check(res[last()] == null);
>>>
>>> // Check less elements than expected, but more than a.length
>>> a = new Object[CAP-2]; // appears too small
>>> CANDIDATE.setSizeSequence(CAP, CAP-1);
>>> res = CANDIDATE.toArray(a);
>>> check(res != a);
>>> check(res.length == CAP-1);
>>> check(res[LAST-1] != null);
>>>
>>> // Check less elements than expected, but equal as a.length
>>> a = Arrays.copyOf(OBJECTS, CAP); // appears to match
>>> CANDIDATE.setSizeSequence(CAP, CAP-1);
>>> res = CANDIDATE.toArray(a);
>>> check(res == a);
>>> check(res[last()] == null);
>>>
>>> // Check more elements than expected and more than a.length
>>> a = new Object[CAP-1]; // appears to match
>>> CANDIDATE.setSizeSequence(CAP-****1, CAP);
>>>
>>> res = CANDIDATE.toArray(a);
>>> check(res != a);
>>> check(res[LAST] != null);
>>>
>>> // Check more elements than expected, but equal as a.length
>>> a = new Object[CAP-1]; // appears to match
>>> CANDIDATE.setSizeSequence(CAP-****2, CAP-1);
>>>
>>> res = CANDIDATE.toArray(a);
>>> check(res == a);
>>> check(res[last()] != null);
>>>
>>> // Check more elements than expected, but less than a.length
>>> a = Arrays.copyOf(OBJECTS, CAP); // appears to match
>>> CANDIDATE.setSizeSequence(CAP-****2, CAP-1);
>>>
>>> res = CANDIDATE.toArray(a);
>>> check(res == a);
>>> check(res[last()] == null);
>>>
>>> test_7121314();
>>> }
>>>
>>> /*
>>> * Major target of this testcase, bug 7121314.
>>> */
>>> protected void test_7121314() throws Throwable {
>>> // Check equal elements as a.length, but less than expected
>>> a = new Object[CAP-1]; // appears too small
>>> CANDIDATE.setSizeSequence(CAP, CAP-1);
>>> res = CANDIDATE.toArray(a);
>>> check(res == a);
>>> check(res[last()] != null);
>>>
>>> // Check less elements than a.length and less than expected
>>> a = Arrays.copyOf(OBJECTS, CAP-1); // appears too small
>>> CANDIDATE.setSizeSequence(CAP, CAP-2);
>>> res = CANDIDATE.toArray(a);
>>> check(res == a);
>>> check(res[last()] == null);
>>>
>>> }
>>>
>>>
>>> public static void main(String[] args) throws Throwable {
>>> ToArrayTest testcase = new ToArrayTest();
>>> try { testcase.test(); } catch (Throwable t) { unexpected(t); }
>>>
>>> System.out.printf("%nPassed = %d, failed = %d%n%n", passed, failed);
>>> if (failed> 0) throw new Exception("Some tests failed");
>>> }
>>>
>>> private static volatile int passed = 0, failed = 0;
>>> private static void pass() { passed++; }
>>> private static void fail() { failed++; Thread.dumpStack(); }
>>> private static void fail(String msg) { System.out.println(msg); fail();
>>> }
>>> private static void unexpected(Throwable t) { failed++;
>>> t.printStackTrace(); }
>>>
>>> static void check(boolean cond) { if (cond) pass(); else fail(); }
>>> static void equals(Object x, Object y) {
>>>
>>> if (x == null ? y == null : x.equals(y)) pass();
>>> else {System.out.println(x + " not equal to " + y); fail(); }}
>>>
>>> }
>>>
>>>
>>>
>>>
>>
>
> --
> Yours Charles
>
>
--
Best Regards,
Sean Chou
