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.hol...@oracle.com
<mailto: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>
<mailto: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/>
<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
