Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec
Joe Darcy
joe.darcy at oracle.com
Mon Apr 9 16:44:22 UTC 2012
The "Contributed-by:" information has to be email addresses:
http://openjdk.java.net/guide/producingChangeset.html
HTH,
-Joe
On 04/08/2012 06:57 PM, Charles Lee wrote:
> Hi guys,
>
> I am trying to push the changeset to the gate, but no matter how I
> change the Contributed-by list, I always get:
>
> /searching for changes
> remote: adding changesets
> remote: adding manifests
> remote: adding file changes
> remote: added 1 changesets with 2 changes to 2 files
> remote: [jcheck a54aa58bfef9 2012-02-26 07:41 -0800]
> remote:
> remote: > Changeset: 5201:c8ef92791b47
> remote: > Author: littlee
> remote: > Date: 2012-04-09 09:51
> remote: >
> remote: > 7121314: Behavior mismatch between
> AbstractCollection.toArray(T[] ) and its spec
> remote: > Reviewed-by: dholmes, mduigou
> remote: > Contributed-by: Sean Zhou, Ulf Zibis, David Holmes
> remote:
> remote: Invalid contributor attribution/
>
>
> I have tried:
> a. name <email> pattern
> b. name, name, name
> c. zhouyx, dhomes, Ulf Zibis
>
> Does anybody have some experience on this?
>
> On 04/01/2012 09:14 PM, David Holmes wrote:
>> Thanks Sean. I'd say this is done.
>>
>> David
>>
>> On 1/04/2012 7: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/ .
>>>
>>>
>>> The link to the bug:
>>> 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
>>>
>>>
>>>
>>> On Sun, Apr 1, 2012 at 11:25 AM, David Holmes <david.holmes at oracle.com
>>> <mailto:david.holmes at 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.Zibis at gmx.de
>>> <mailto:Ulf.Zibis at gmx.de>
>>> <mailto:Ulf.Zibis at gmx.de <mailto:Ulf.Zibis at 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/>
>>> <http://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/>
>>>
>>> <http://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>>
>>> 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>
>>>
>>>
>>> <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 <http://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(); }}
>>>
>>> }
>>>
>>>
>>>
>>>
>>>
>>> --
>>> Best Regards,
>>> Sean Chou
>>>
>>
>
>
More information about the core-libs-dev
mailing list