8146568: NegativeArraySizeException in ArrayList.grow(int)
Summary: improve management of internal array
Reviewed-by: smarks
--- a/jdk/src/java.base/share/classes/java/util/ArrayList.java Fri Jan 22 12:44:32 2016 -0800
+++ b/jdk/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/jdk/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/jdk/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) {}
+ }
+}