From 4a29f05c6ad61db2932750d9799a050e6954bf04 Mon Sep 17 00:00:00 2001 From: Martin Buchholz Date: Sun, 9 May 2010 00:59:30 -0700 Subject: [PATCH] 6933217: Huge arrays handled poorly in core libraries Write overflow-conscious array resizing code Reviewed-by: chegar --- .../java/io/ByteArrayOutputStream.java | 56 ++++++-- .../java/lang/AbstractStringBuilder.java | 124 +++++++----------- .../classes/java/util/AbstractCollection.java | 28 +++- .../share/classes/java/util/ArrayList.java | 47 +++++-- .../share/classes/java/util/Hashtable.java | 17 ++- .../classes/java/util/PriorityQueue.java | 31 +++-- jdk/src/share/classes/java/util/Vector.java | 39 ++++-- 7 files changed, 218 insertions(+), 124 deletions(-) diff --git a/jdk/src/share/classes/java/io/ByteArrayOutputStream.java b/jdk/src/share/classes/java/io/ByteArrayOutputStream.java index 6a9b190f20c..a3b0e06bbb4 100644 --- a/jdk/src/share/classes/java/io/ByteArrayOutputStream.java +++ b/jdk/src/share/classes/java/io/ByteArrayOutputStream.java @@ -77,18 +77,51 @@ public class ByteArrayOutputStream extends OutputStream { buf = new byte[size]; } + /** + * Increases the capacity if 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 + * @throws OutOfMemoryError if {@code minCapacity < 0}. This is + * interpreted as a request for the unsatisfiably large capacity + * {@code (long) Integer.MAX_VALUE + (minCapacity - Integer.MAX_VALUE)}. + */ + private void ensureCapacity(int minCapacity) { + // overflow-conscious code + if (minCapacity - buf.length > 0) + grow(minCapacity); + } + + /** + * Increases the capacity to ensure that it can hold at least the + * number of elements specified by the minimum capacity argument. + * + * @param minCapacity the desired minimum capacity + */ + private void grow(int minCapacity) { + // overflow-conscious code + int oldCapacity = buf.length; + int newCapacity = oldCapacity << 1; + if (newCapacity - minCapacity < 0) + newCapacity = minCapacity; + if (newCapacity < 0) { + if (minCapacity < 0) // overflow + throw new OutOfMemoryError(); + newCapacity = Integer.MAX_VALUE; + } + buf = Arrays.copyOf(buf, newCapacity); + } + /** * Writes the specified byte to this byte array output stream. * * @param b the byte to be written. */ public synchronized void write(int b) { - int newcount = count + 1; - if (newcount > buf.length) { - buf = Arrays.copyOf(buf, Math.max(buf.length << 1, newcount)); - } - buf[count] = (byte)b; - count = newcount; + ensureCapacity(count + 1); + buf[count] = (byte) b; + count += 1; } /** @@ -101,17 +134,12 @@ public class ByteArrayOutputStream extends OutputStream { */ public synchronized void write(byte b[], int off, int len) { if ((off < 0) || (off > b.length) || (len < 0) || - ((off + len) > b.length) || ((off + len) < 0)) { + ((off + len) - b.length > 0)) { throw new IndexOutOfBoundsException(); - } else if (len == 0) { - return; - } - int newcount = count + len; - if (newcount > buf.length) { - buf = Arrays.copyOf(buf, Math.max(buf.length << 1, newcount)); } + ensureCapacity(count + len); System.arraycopy(b, off, buf, count, len); - count = newcount; + count += len; } /** diff --git a/jdk/src/share/classes/java/lang/AbstractStringBuilder.java b/jdk/src/share/classes/java/lang/AbstractStringBuilder.java index 973e0854d61..fd98eb0403c 100644 --- a/jdk/src/share/classes/java/lang/AbstractStringBuilder.java +++ b/jdk/src/share/classes/java/lang/AbstractStringBuilder.java @@ -36,6 +36,8 @@ import java.util.Arrays; * sequence can be changed through certain method calls. * * @author Michael McCloskey + * @author Martin Buchholz + * @author Ulf Zibis * @since 1.5 */ abstract class AbstractStringBuilder implements Appendable, CharSequence { @@ -98,9 +100,16 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { * @param minimumCapacity the minimum desired capacity. */ public void ensureCapacity(int minimumCapacity) { - if (minimumCapacity > value.length) { + ensureCapacityInternal(minimumCapacity); + } + + /** + * This method has the same contract as ensureCapacity, but is + * never synchronized. + */ + private void ensureCapacityInternal(int minimumCapacity) { + if (minimumCapacity - value.length > 0) expandCapacity(minimumCapacity); - } } /** @@ -108,11 +117,13 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { * size check or synchronization. */ void expandCapacity(int minimumCapacity) { - int newCapacity = (value.length + 1) * 2; - if (newCapacity < 0) { - newCapacity = Integer.MAX_VALUE; - } else if (minimumCapacity > newCapacity) { + int newCapacity = value.length * 2; + if (newCapacity - minimumCapacity < 0) newCapacity = minimumCapacity; + if (newCapacity < 0) { + if (minimumCapacity < 0) // overflow + throw new OutOfMemoryError(); + newCapacity = Integer.MAX_VALUE; } value = Arrays.copyOf(value, newCapacity); } @@ -158,8 +169,7 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { public void setLength(int newLength) { if (newLength < 0) throw new StringIndexOutOfBoundsException(newLength); - if (newLength > value.length) - expandCapacity(newLength); + ensureCapacityInternal(newLength); if (count < newLength) { for (; count < newLength; count++) @@ -400,12 +410,9 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { public AbstractStringBuilder append(String str) { if (str == null) str = "null"; int len = str.length(); - if (len == 0) return this; - int newCount = count + len; - if (newCount > value.length) - expandCapacity(newCount); + ensureCapacityInternal(count + len); str.getChars(0, len, value, count); - count = newCount; + count += len; return this; } @@ -414,11 +421,9 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { if (sb == null) return append("null"); int len = sb.length(); - int newCount = count + len; - if (newCount > value.length) - expandCapacity(newCount); + ensureCapacityInternal(count + len); sb.getChars(0, len, value, count); - count = newCount; + count += len; return this; } @@ -470,14 +475,10 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { "start " + start + ", end " + end + ", s.length() " + s.length()); int len = end - start; - if (len == 0) - return this; - int newCount = count + len; - if (newCount > value.length) - expandCapacity(newCount); - for (int i=start; i value.length) - expandCapacity(newCount); - System.arraycopy(str, 0, value, count, str.length); - count = newCount; + int len = str.length; + ensureCapacityInternal(count + len); + System.arraycopy(str, 0, value, count, len); + count += len; return this; } @@ -529,11 +529,9 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { * or {@code offset+len > str.length} */ public AbstractStringBuilder append(char str[], int offset, int len) { - int newCount = count + len; - if (newCount > value.length) - expandCapacity(newCount); + ensureCapacityInternal(count + len); System.arraycopy(str, offset, value, count, len); - count = newCount; + count += len; return this; } @@ -551,17 +549,13 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { */ public AbstractStringBuilder append(boolean b) { if (b) { - int newCount = count + 4; - if (newCount > value.length) - expandCapacity(newCount); + ensureCapacityInternal(count + 4); value[count++] = 't'; value[count++] = 'r'; value[count++] = 'u'; value[count++] = 'e'; } else { - int newCount = count + 5; - if (newCount > value.length) - expandCapacity(newCount); + ensureCapacityInternal(count + 5); value[count++] = 'f'; value[count++] = 'a'; value[count++] = 'l'; @@ -587,9 +581,7 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { * @return a reference to this object. */ public AbstractStringBuilder append(char c) { - int newCount = count + 1; - if (newCount > value.length) - expandCapacity(newCount); + ensureCapacityInternal(count + 1); value[count++] = c; return this; } @@ -614,8 +606,7 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { int appendedLength = (i < 0) ? Integer.stringSize(-i) + 1 : Integer.stringSize(i); int spaceNeeded = count + appendedLength; - if (spaceNeeded > value.length) - expandCapacity(spaceNeeded); + ensureCapacityInternal(spaceNeeded); Integer.getChars(i, spaceNeeded, value); count = spaceNeeded; return this; @@ -641,8 +632,7 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { int appendedLength = (l < 0) ? Long.stringSize(-l) + 1 : Long.stringSize(l); int spaceNeeded = count + appendedLength; - if (spaceNeeded > value.length) - expandCapacity(spaceNeeded); + ensureCapacityInternal(spaceNeeded); Long.getChars(l, spaceNeeded, value); count = spaceNeeded; return this; @@ -738,10 +728,7 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { if (codePoint >= Character.MIN_SUPPLEMENTARY_CODE_POINT) { n++; } - int newCount = count + n; - if (newCount > value.length) { - expandCapacity(newCount); - } + ensureCapacityInternal(count + n); if (n == 1) { value[count++] = (char) codePoint; } else { @@ -807,8 +794,7 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { end = count; int len = str.length(); int newCount = count + len - (end - start); - if (newCount > value.length) - expandCapacity(newCount); + ensureCapacityInternal(newCount); System.arraycopy(value, end, value, start + len, count - end); str.getChars(value, start); @@ -915,12 +901,10 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { throw new StringIndexOutOfBoundsException( "offset " + offset + ", len " + len + ", str.length " + str.length); - int newCount = count + len; - if (newCount > value.length) - expandCapacity(newCount); + ensureCapacityInternal(count + len); System.arraycopy(value, index, value, index + len, count - index); System.arraycopy(str, offset, value, index, len); - count = newCount; + count += len; return this; } @@ -984,12 +968,10 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { if (str == null) str = "null"; int len = str.length(); - int newCount = count + len; - if (newCount > value.length) - expandCapacity(newCount); + ensureCapacityInternal(count + len); System.arraycopy(value, offset, value, offset + len, count - offset); str.getChars(value, offset); - count = newCount; + count += len; return this; } @@ -1021,12 +1003,10 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { if ((offset < 0) || (offset > length())) throw new StringIndexOutOfBoundsException(offset); int len = str.length; - int newCount = count + len; - if (newCount > value.length) - expandCapacity(newCount); + ensureCapacityInternal(count + len); System.arraycopy(value, offset, value, offset + len, count - offset); System.arraycopy(str, 0, value, offset, len); - count = newCount; + count += len; return this; } @@ -1114,16 +1094,12 @@ abstract class AbstractStringBuilder implements Appendable, CharSequence { "start " + start + ", end " + end + ", s.length() " + s.length()); int len = end - start; - if (len == 0) - return this; - int newCount = count + len; - if (newCount > value.length) - expandCapacity(newCount); + ensureCapacityInternal(count + len); System.arraycopy(value, dstOffset, value, dstOffset + len, count - dstOffset); for (int i=start; i value.length) - expandCapacity(newCount); + ensureCapacityInternal(count + 1); System.arraycopy(value, offset, value, offset + 1, count - offset); value[offset] = c; - count = newCount; + count += 1; return this; } diff --git a/jdk/src/share/classes/java/util/AbstractCollection.java b/jdk/src/share/classes/java/util/AbstractCollection.java index 5b13c9b00fd..f40d050589f 100644 --- a/jdk/src/share/classes/java/util/AbstractCollection.java +++ b/jdk/src/share/classes/java/util/AbstractCollection.java @@ -190,6 +190,14 @@ public abstract class AbstractCollection implements Collection { return it.hasNext() ? finishToArray(r, it) : r; } + /** + * The maximum size of array to allocate. + * Some VMs reserve some header words in an array. + * Attempts to allocate larger arrays may result in + * OutOfMemoryError: Requested array size exceeds VM limit + */ + private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; + /** * Reallocates the array being used within toArray when the iterator * returned more elements than expected, and finishes filling it from @@ -205,13 +213,10 @@ public abstract class AbstractCollection implements Collection { while (it.hasNext()) { int cap = r.length; if (i == cap) { - int newCap = ((cap / 2) + 1) * 3; - if (newCap <= cap) { // integer overflow - if (cap == Integer.MAX_VALUE) - throw new OutOfMemoryError - ("Required array size too large"); - newCap = Integer.MAX_VALUE; - } + int newCap = cap + (cap >> 1) + 1; + // overflow-conscious code + if (newCap - MAX_ARRAY_SIZE > 0) + newCap = hugeCapacity(cap + 1); r = Arrays.copyOf(r, newCap); } r[i++] = (T)it.next(); @@ -220,6 +225,15 @@ public abstract class AbstractCollection implements Collection { return (i == r.length) ? r : Arrays.copyOf(r, i); } + private static int hugeCapacity(int minCapacity) { + if (minCapacity < 0) // overflow + throw new OutOfMemoryError + ("Required array size too large"); + return (minCapacity > MAX_ARRAY_SIZE) ? + Integer.MAX_VALUE : + MAX_ARRAY_SIZE; + } + // Modification Operations /** diff --git a/jdk/src/share/classes/java/util/ArrayList.java b/jdk/src/share/classes/java/util/ArrayList.java index dbd46096bf3..92dbfd9c84b 100644 --- a/jdk/src/share/classes/java/util/ArrayList.java +++ b/jdk/src/share/classes/java/util/ArrayList.java @@ -173,18 +173,47 @@ public class ArrayList extends AbstractList * 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) { modCount++; + // overflow-conscious code + if (minCapacity - elementData.length > 0) + grow(minCapacity); + } + + /** + * The maximum size of array to allocate. + * Some VMs reserve some header words in an array. + * Attempts to allocate larger arrays may result in + * OutOfMemoryError: Requested array size exceeds VM limit + */ + private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; + + /** + * Increases the capacity to ensure that it can hold at least the + * number of elements specified by the minimum capacity argument. + * + * @param minCapacity the desired minimum capacity + */ + private void grow(int minCapacity) { + // overflow-conscious code int oldCapacity = elementData.length; - if (minCapacity > oldCapacity) { - int newCapacity = (oldCapacity * 3)/2 + 1; - if (newCapacity < minCapacity) - newCapacity = minCapacity; - // minCapacity is usually close to size, so this is a win: - elementData = Arrays.copyOf(elementData, newCapacity); - } + 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); + } + + private static int hugeCapacity(int minCapacity) { + if (minCapacity < 0) // overflow + throw new OutOfMemoryError(); + return (minCapacity > MAX_ARRAY_SIZE) ? + Integer.MAX_VALUE : + MAX_ARRAY_SIZE; } /** @@ -391,7 +420,7 @@ public class ArrayList extends AbstractList public void add(int index, E element) { rangeCheckForAdd(index); - ensureCapacity(size+1); // Increments modCount!! + ensureCapacity(size + 1); // Increments modCount!! System.arraycopy(elementData, index, elementData, index + 1, size - index); elementData[index] = element; diff --git a/jdk/src/share/classes/java/util/Hashtable.java b/jdk/src/share/classes/java/util/Hashtable.java index 4fbd336ca6c..17267985d2a 100644 --- a/jdk/src/share/classes/java/util/Hashtable.java +++ b/jdk/src/share/classes/java/util/Hashtable.java @@ -364,6 +364,14 @@ public class Hashtable return null; } + /** + * The maximum size of array to allocate. + * Some VMs reserve some header words in an array. + * Attempts to allocate larger arrays may result in + * OutOfMemoryError: Requested array size exceeds VM limit + */ + private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; + /** * Increases the capacity of and internally reorganizes this * hashtable, in order to accommodate and access its entries more @@ -375,7 +383,14 @@ public class Hashtable int oldCapacity = table.length; Entry[] oldMap = table; - int newCapacity = oldCapacity * 2 + 1; + // overflow-conscious code + int newCapacity = (oldCapacity << 1) + 1; + if (newCapacity - MAX_ARRAY_SIZE > 0) { + if (oldCapacity == MAX_ARRAY_SIZE) + // Keep running with MAX_ARRAY_SIZE buckets + return; + newCapacity = MAX_ARRAY_SIZE; + } Entry[] newMap = new Entry[newCapacity]; modCount++; diff --git a/jdk/src/share/classes/java/util/PriorityQueue.java b/jdk/src/share/classes/java/util/PriorityQueue.java index 428792fe1c6..ac87d9fb569 100644 --- a/jdk/src/share/classes/java/util/PriorityQueue.java +++ b/jdk/src/share/classes/java/util/PriorityQueue.java @@ -235,26 +235,39 @@ public class PriorityQueue extends AbstractQueue size = a.length; } + /** + * The maximum size of array to allocate. + * Some VMs reserve some header words in an array. + * Attempts to allocate larger arrays may result in + * OutOfMemoryError: Requested array size exceeds VM limit + */ + private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; + /** * Increases the capacity of the array. * * @param minCapacity the desired minimum capacity */ private void grow(int minCapacity) { - if (minCapacity < 0) // overflow - throw new OutOfMemoryError(); int oldCapacity = queue.length; // Double size if small; else grow by 50% - int newCapacity = ((oldCapacity < 64)? - ((oldCapacity + 1) * 2): - ((oldCapacity / 2) * 3)); - if (newCapacity < 0) // overflow - newCapacity = Integer.MAX_VALUE; - if (newCapacity < minCapacity) - newCapacity = minCapacity; + int newCapacity = oldCapacity + ((oldCapacity < 64) ? + (oldCapacity + 2) : + (oldCapacity >> 1)); + // overflow-conscious code + if (newCapacity - MAX_ARRAY_SIZE > 0) + newCapacity = hugeCapacity(minCapacity); queue = Arrays.copyOf(queue, newCapacity); } + private static int hugeCapacity(int minCapacity) { + if (minCapacity < 0) // overflow + throw new OutOfMemoryError(); + return (minCapacity > MAX_ARRAY_SIZE) ? + Integer.MAX_VALUE : + MAX_ARRAY_SIZE; + } + /** * Inserts the specified element into this priority queue. * diff --git a/jdk/src/share/classes/java/util/Vector.java b/jdk/src/share/classes/java/util/Vector.java index f70d36ee069..6fa6c84f8da 100644 --- a/jdk/src/share/classes/java/util/Vector.java +++ b/jdk/src/share/classes/java/util/Vector.java @@ -235,16 +235,37 @@ public class Vector * @see #ensureCapacity(int) */ private void ensureCapacityHelper(int minCapacity) { + // overflow-conscious code + if (minCapacity - elementData.length > 0) + grow(minCapacity); + } + + /** + * The maximum size of array to allocate. + * Some VMs reserve some header words in an array. + * Attempts to allocate larger arrays may result in + * OutOfMemoryError: Requested array size exceeds VM limit + */ + private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; + + private void grow(int minCapacity) { + // overflow-conscious code int oldCapacity = elementData.length; - if (minCapacity > oldCapacity) { - Object[] oldData = elementData; - int newCapacity = (capacityIncrement > 0) ? - (oldCapacity + capacityIncrement) : (oldCapacity * 2); - if (newCapacity < minCapacity) { - newCapacity = minCapacity; - } - elementData = Arrays.copyOf(elementData, newCapacity); - } + int newCapacity = oldCapacity + ((capacityIncrement > 0) ? + capacityIncrement : oldCapacity); + if (newCapacity - minCapacity < 0) + newCapacity = minCapacity; + if (newCapacity - MAX_ARRAY_SIZE > 0) + newCapacity = hugeCapacity(minCapacity); + elementData = Arrays.copyOf(elementData, newCapacity); + } + + private static int hugeCapacity(int minCapacity) { + if (minCapacity < 0) // overflow + throw new OutOfMemoryError(); + return (minCapacity > MAX_ARRAY_SIZE) ? + Integer.MAX_VALUE : + MAX_ARRAY_SIZE; } /**