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

David Holmes david.holmes at oracle.com
Sun Apr 1 03:25:59 UTC 2012


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>> 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/%__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>
>         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>
>
>     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(); }}

}





More information about the core-libs-dev mailing list