Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec
Charles Lee
littlee at linux.vnet.ibm.com
Mon Apr 9 01:57:17 UTC 2012
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
>>
>
--
Yours Charles
More information about the core-libs-dev
mailing list