changeset 13542:afa1a4e34c20

8146568: NegativeArraySizeException in ArrayList.grow(int) Summary: improve management of internal array Reviewed-by: smarks
author martin
date Fri, 08 Jan 2016 19:53:36 -0800
parents 7f5b7acebffd
children 45ae2c3e1d52
files src/java.base/share/classes/java/util/ArrayList.java test/java/util/ArrayList/ArrayManagement.java test/java/util/ArrayList/Bug8146568.java
diffstat 3 files changed, 350 insertions(+), 60 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/util/ArrayList.java	Fri Jan 22 12:44:32 2016 -0800
+++ b/src/java.base/share/classes/java/util/ArrayList.java	Fri Jan 08 19:53:36 2016 -0800
@@ -207,39 +207,19 @@
      * necessary, to ensure that it can hold at least the number of elements
      * specified by the minimum capacity argument.
      *
-     * @param   minCapacity   the desired minimum capacity
+     * @param minCapacity the desired minimum capacity
      */
     public void ensureCapacity(int minCapacity) {
-        int minExpand = (elementData != DEFAULTCAPACITY_EMPTY_ELEMENTDATA)
-            // any size if not default element table
-            ? 0
-            // larger than default for default empty table. It's already
-            // supposed to be at default size.
-            : DEFAULT_CAPACITY;
-
-        if (minCapacity > minExpand) {
-            ensureExplicitCapacity(minCapacity);
+        if (minCapacity > elementData.length
+            && !(elementData == DEFAULTCAPACITY_EMPTY_ELEMENTDATA
+                 && minCapacity <= DEFAULT_CAPACITY)) {
+            modCount++;
+            grow(minCapacity);
         }
     }
 
-    private void ensureCapacityInternal(int minCapacity) {
-        if (elementData == DEFAULTCAPACITY_EMPTY_ELEMENTDATA) {
-            minCapacity = Math.max(DEFAULT_CAPACITY, minCapacity);
-        }
-
-        ensureExplicitCapacity(minCapacity);
-    }
-
-    private void ensureExplicitCapacity(int minCapacity) {
-        modCount++;
-
-        // overflow-conscious code
-        if (minCapacity - elementData.length > 0)
-            grow(minCapacity);
-    }
-
     /**
-     * The maximum size of array to allocate.
+     * The maximum size of array to allocate (unless necessary).
      * Some VMs reserve some header words in an array.
      * Attempts to allocate larger arrays may result in
      * OutOfMemoryError: Requested array size exceeds VM limit
@@ -251,25 +231,48 @@
      * number of elements specified by the minimum capacity argument.
      *
      * @param minCapacity the desired minimum capacity
+     * @throws OutOfMemoryError if minCapacity is less than zero
      */
-    private void grow(int minCapacity) {
+    private Object[] grow(int minCapacity) {
+        return elementData = Arrays.copyOf(elementData,
+                                           newCapacity(minCapacity));
+    }
+
+    private Object[] grow() {
+        return grow(size + 1);
+    }
+
+    /**
+     * Returns a capacity at least as large as the given minimum capacity.
+     * Returns the current capacity increased by 50% if that suffices.
+     * Will not return a capacity greater than MAX_ARRAY_SIZE unless
+     * the given minimum capacity is greater than MAX_ARRAY_SIZE.
+     *
+     * @param minCapacity the desired minimum capacity
+     * @throws OutOfMemoryError if minCapacity is less than zero
+     */
+    private int newCapacity(int minCapacity) {
         // overflow-conscious code
         int oldCapacity = elementData.length;
         int newCapacity = oldCapacity + (oldCapacity >> 1);
-        if (newCapacity - minCapacity < 0)
-            newCapacity = minCapacity;
-        if (newCapacity - MAX_ARRAY_SIZE > 0)
-            newCapacity = hugeCapacity(minCapacity);
-        // minCapacity is usually close to size, so this is a win:
-        elementData = Arrays.copyOf(elementData, newCapacity);
+        if (newCapacity - minCapacity <= 0) {
+            if (elementData == DEFAULTCAPACITY_EMPTY_ELEMENTDATA)
+                return Math.max(DEFAULT_CAPACITY, minCapacity);
+            if (minCapacity < 0) // overflow
+                throw new OutOfMemoryError();
+            return minCapacity;
+        }
+        return (newCapacity - MAX_ARRAY_SIZE <= 0)
+            ? newCapacity
+            : hugeCapacity(minCapacity);
     }
 
     private static int hugeCapacity(int minCapacity) {
         if (minCapacity < 0) // overflow
             throw new OutOfMemoryError();
-        return (minCapacity > MAX_ARRAY_SIZE) ?
-            Integer.MAX_VALUE :
-            MAX_ARRAY_SIZE;
+        return (minCapacity > MAX_ARRAY_SIZE)
+            ? Integer.MAX_VALUE
+            : MAX_ARRAY_SIZE;
     }
 
     /**
@@ -452,14 +455,26 @@
     }
 
     /**
+     * This helper method split out from add(E) to keep method
+     * bytecode size under 35 (the -XX:MaxInlineSize default value),
+     * which helps when add(E) is called in a C1-compiled loop.
+     */
+    private void add(E e, Object[] elementData, int s) {
+        if (s == elementData.length)
+            elementData = grow();
+        elementData[s] = e;
+        size = s + 1;
+    }
+
+    /**
      * Appends the specified element to the end of this list.
      *
      * @param e element to be appended to this list
      * @return {@code true} (as specified by {@link Collection#add})
      */
     public boolean add(E e) {
-        ensureCapacityInternal(size + 1);  // Increments modCount!!
-        elementData[size++] = e;
+        modCount++;
+        add(e, elementData, size);
         return true;
     }
 
@@ -474,12 +489,16 @@
      */
     public void add(int index, E element) {
         rangeCheckForAdd(index);
-
-        ensureCapacityInternal(size + 1);  // Increments modCount!!
-        System.arraycopy(elementData, index, elementData, index + 1,
-                         size - index);
+        modCount++;
+        final int s;
+        Object[] elementData;
+        if ((s = size) == (elementData = this.elementData).length)
+            elementData = grow();
+        System.arraycopy(elementData, index,
+                         elementData, index + 1,
+                         s - index);
         elementData[index] = element;
-        size++;
+        size = s + 1;
     }
 
     /**
@@ -578,11 +597,17 @@
      */
     public boolean addAll(Collection<? extends E> c) {
         Object[] a = c.toArray();
+        modCount++;
         int numNew = a.length;
-        ensureCapacityInternal(size + numNew);  // Increments modCount
-        System.arraycopy(a, 0, elementData, size, numNew);
-        size += numNew;
-        return numNew != 0;
+        if (numNew == 0)
+            return false;
+        Object[] elementData;
+        final int s;
+        if (numNew > (elementData = this.elementData).length - (s = size))
+            elementData = grow(s + numNew);
+        System.arraycopy(a, 0, elementData, s, numNew);
+        size = s + numNew;
+        return true;
     }
 
     /**
@@ -604,17 +629,23 @@
         rangeCheckForAdd(index);
 
         Object[] a = c.toArray();
+        modCount++;
         int numNew = a.length;
-        ensureCapacityInternal(size + numNew);  // Increments modCount
+        if (numNew == 0)
+            return false;
+        Object[] elementData;
+        final int s;
+        if (numNew > (elementData = this.elementData).length - (s = size))
+            elementData = grow(s + numNew);
 
-        int numMoved = size - index;
+        int numMoved = s - index;
         if (numMoved > 0)
-            System.arraycopy(elementData, index, elementData, index + numNew,
+            System.arraycopy(elementData, index,
+                             elementData, index + numNew,
                              numMoved);
-
         System.arraycopy(a, 0, elementData, index, numNew);
-        size += numNew;
-        return numNew != 0;
+        size = s + numNew;
+        return true;
     }
 
     /**
@@ -786,7 +817,6 @@
      */
     private void readObject(java.io.ObjectInputStream s)
         throws java.io.IOException, ClassNotFoundException {
-        elementData = EMPTY_ELEMENTDATA;
 
         // Read in size, and any hidden stuff
         s.defaultReadObject();
@@ -795,14 +825,19 @@
         s.readInt(); // ignored
 
         if (size > 0) {
-            // be like clone(), allocate array based upon size not capacity
-            ensureCapacityInternal(size);
+            // like clone(), allocate array based upon size not capacity
+            Object[] elements = new Object[size];
 
-            Object[] a = elementData;
             // Read in all elements in the proper order.
-            for (int i=0; i<size; i++) {
-                a[i] = s.readObject();
+            for (int i = 0; i < size; i++) {
+                elements[i] = s.readObject();
             }
+
+            elementData = elements;
+        } else if (size == 0) {
+            elementData = EMPTY_ELEMENTDATA;
+        } else {
+            throw new java.io.InvalidObjectException("Invalid size: " + size);
         }
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/java/util/ArrayList/ArrayManagement.java	Fri Jan 08 19:53:36 2016 -0800
@@ -0,0 +1,212 @@
+/*
+ * Copyright 2016 Google, Inc.  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 8146568
+ * @summary brittle white box test of internal array management
+ * @run testng ArrayManagement
+ */
+
+import java.lang.reflect.Field;
+import java.util.AbstractList;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.SplittableRandom;
+
+import org.testng.annotations.Test;
+import static org.testng.Assert.*;
+
+public class ArrayManagement {
+    static final int DEFAULT_CAPACITY = 10;
+    static final Field ELEMENT_DATA;
+    static final Field MODCOUNT;
+    static final SplittableRandom rnd = new SplittableRandom();
+
+    static {
+        try {
+            ELEMENT_DATA = ArrayList.class.getDeclaredField("elementData");
+            ELEMENT_DATA.setAccessible(true);
+            MODCOUNT = AbstractList.class.getDeclaredField("modCount");
+            MODCOUNT.setAccessible(true);
+        } catch (ReflectiveOperationException huh) {
+            throw new AssertionError(huh);
+        }
+    }
+
+    static Object[] elementData(ArrayList<?> list) {
+        try {
+            return (Object[]) ELEMENT_DATA.get(list);
+        } catch (ReflectiveOperationException huh) {
+            throw new AssertionError(huh);
+        }
+    }
+
+    static int modCount(ArrayList<?> list) {
+        try {
+            return MODCOUNT.getInt(list);
+        } catch (ReflectiveOperationException huh) {
+            throw new AssertionError(huh);
+        }
+    }
+
+    static int capacity(ArrayList<?> list) {
+        return elementData(list).length;
+    }
+
+    static int newCapacity(int oldCapacity) {
+        return oldCapacity + (oldCapacity >> 1);
+    }
+
+    static void ensureCapacity(ArrayList<Object> list, int capacity) {
+        int oldCapacity = capacity(list);
+        int oldModCount = modCount(list);
+        list.ensureCapacity(capacity);
+        assertEquals(modCount(list),
+                     (capacity(list) == oldCapacity)
+                     ? oldModCount
+                     : oldModCount + 1);
+    }
+
+    static List<Object> singletonList() {
+        return Collections.singletonList(Boolean.TRUE);
+    }
+
+    /** Opportunistically randomly test various add operations. */
+    static void addOneElement(ArrayList<Object> list) {
+        int size = list.size();
+        int modCount = modCount(list);
+        switch (rnd.nextInt(4)) {
+        case 0: assertTrue(list.add(Boolean.TRUE)); break;
+        case 1: list.add(size, Boolean.TRUE); break;
+        case 2: assertTrue(list.addAll(singletonList())); break;
+        case 3: assertTrue(list.addAll(size, singletonList())); break;
+        default: throw new AssertionError();
+        }
+        assertEquals(modCount(list), modCount + 1);
+        assertEquals(list.size(), size + 1);
+    }
+
+    @Test public void defaultCapacity() {
+        ArrayList<Object> list = new ArrayList<>();
+        assertEquals(capacity(new ArrayList<Object>()), 0);
+        for (int i = 0; i < DEFAULT_CAPACITY; i++) {
+            addOneElement(list);
+            assertEquals(capacity(list), DEFAULT_CAPACITY);
+        }
+        addOneElement(list);
+        assertEquals(capacity(list), newCapacity(DEFAULT_CAPACITY));
+    }
+
+    @Test public void defaultCapacityEnsureCapacity() {
+        ArrayList<Object> list = new ArrayList<>();
+        for (int i = 0; i <= DEFAULT_CAPACITY; i++) {
+            ensureCapacity(list, i);     // no-op!
+            assertEquals(capacity(list), 0);
+        }
+        for (int i = 0; i < DEFAULT_CAPACITY; i++) {
+            addOneElement(list);
+            assertEquals(capacity(list), DEFAULT_CAPACITY);
+        }
+        addOneElement(list);
+        assertEquals(capacity(list), newCapacity(DEFAULT_CAPACITY));
+        {
+            int capacity = capacity(list);
+            ensureCapacity(list, capacity + 1);
+            assertEquals(capacity(list), newCapacity(capacity));
+        }
+        {
+            int capacity = capacity(list);
+            ensureCapacity(list, 3 * capacity);
+            assertEquals(capacity(list), 3 * capacity);
+        }
+    }
+
+    @Test public void ensureCapacityBeyondDefaultCapacity() {
+        ArrayList<Object> list = new ArrayList<>();
+        list.ensureCapacity(DEFAULT_CAPACITY + 1);
+        assertEquals(capacity(list), DEFAULT_CAPACITY + 1);
+        for (int i = 0; i < DEFAULT_CAPACITY + 1; i++) {
+            addOneElement(list);
+            assertEquals(capacity(list), DEFAULT_CAPACITY + 1);
+        }
+        addOneElement(list);
+        assertEquals(capacity(list), newCapacity(DEFAULT_CAPACITY + 1));
+    }
+
+    @Test public void explicitZeroCapacity() {
+        ArrayList<Object> list = new ArrayList<>(0);
+        assertEquals(capacity(list), 0);
+        addOneElement(list);
+        assertEquals(capacity(list), 1);
+        addOneElement(list);
+        assertEquals(capacity(list), 2);
+        addOneElement(list);
+        assertEquals(capacity(list), 3);
+        addOneElement(list);
+        assertEquals(capacity(list), 4);
+        addOneElement(list);
+        assertEquals(capacity(list), 6);
+        addOneElement(list);
+        assertEquals(capacity(list), 6);
+        addOneElement(list);
+        assertEquals(capacity(list), 9);
+        list.clear();
+        assertEquals(capacity(list), 9);
+    }
+
+    @Test public void explicitLargeCapacity() {
+        int n = DEFAULT_CAPACITY * 3;
+        ArrayList<Object> list = new ArrayList<>(n);
+        assertEquals(capacity(list), n);
+        ensureCapacity(list, 0);
+        ensureCapacity(list, n);
+        for (int i = 0; i < n; i++) addOneElement(list);
+        assertEquals(capacity(list), n);
+
+        addOneElement(list);
+        assertEquals(capacity(list), newCapacity(n));
+    }
+
+    @Test public void emptyArraysAreShared() {
+        assertSame(elementData(new ArrayList<Object>()),
+                   elementData(new ArrayList<Object>()));
+        assertSame(elementData(new ArrayList<Object>(0)),
+                   elementData(new ArrayList<Object>(0)));
+    }
+
+    @Test public void emptyArraysDifferBetweenDefaultAndExplicit() {
+        assertNotSame(elementData(new ArrayList<Object>()),
+                      elementData(new ArrayList<Object>(0)));
+    }
+
+    @Test public void negativeCapacity() {
+        for (int capacity : new int[] { -1, Integer.MIN_VALUE }) {
+            try {
+                new ArrayList<Object>(capacity);
+                fail("should throw");
+            } catch (IllegalArgumentException success) {}
+        }
+    }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/java/util/ArrayList/Bug8146568.java	Fri Jan 08 19:53:36 2016 -0800
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2016 Google, Inc.  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 8146568
+ * @summary repro for: NegativeArraySizeException in ArrayList.grow(int)
+ * @run main/othervm -Xmx17g Bug8146568
+ * @ignore This test has huge memory requirements
+ */
+
+public class Bug8146568 {
+    public static void main(String[] args) {
+        int size = Integer.MAX_VALUE - 2;
+        java.util.ArrayList<Object> huge = new java.util.ArrayList<>(size);
+        for (int i = 0; i < size; i++)
+            huge.add(null);
+        try {
+            huge.addAll(huge);
+            throw new Error("expected OutOfMemoryError not thrown");
+        } catch (OutOfMemoryError success) {}
+    }
+}