Sometimes constraints are questionable

[David Holmes]

On 31/05/2020 12:29 am, Jim Laskey wrote:
> I'm working through https://bugs.openjdk.java.net/browse/JDK-8230744 <https://bugs.openjdk.java.net/browse/JDK-8230744> Several classes throw OutOfMemoryError without message .
> 
> I'm wondering why hugeCapacity in src/jdk.zipfs/share/classes/jdk/nio/zipfs/ByteArrayChannel.java is defined as
> 
>      /**
>       * 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 = buf.length;
>          int newCapacity = oldCapacity << 1;
>          if (newCapacity - minCapacity < 0)
>              newCapacity = minCapacity;
>          if (newCapacity - MAX_ARRAY_SIZE > 0)
>              newCapacity = hugeCapacity(minCapacity);
>          buf = Arrays.copyOf(buf, newCapacity);
>      }
> 
>      private static int hugeCapacity(int minCapacity) {
>          if (minCapacity < 0) // overflow
>              throw new OutOfMemoryError();

Not sure how we could have minCapacity < 0 at this point. It should have 
been checked before the call to grow, and grow will not make it negative.

>          return (minCapacity > MAX_ARRAY_SIZE) ?
>              Integer.MAX_VALUE :
>              MAX_ARRAY_SIZE;

That's a bug plain and simple. It should never report a size > 
MAX_ARRAY_SIZE.

>      }
> 
> It just seems that it's pushing the inevitable off to Arrays.copyOf.  Shouldn't it be:
> 
>      private static int hugeCapacity(int minCapacity) {
>          if (minCapacity < 0 || minCapacity > MAX_ARRAY_SIZE) {
>              throw
>                  new OutOfMemoryError("ByteArrayChannel exceeds maximum size: " +
>                                        MAX_ARRAY_SIZE);
>          }
>          
>          return MAX_ARRAY_SIZE;
>      }

That seems more appropriate to me - modulo the question mark over 
minCapacity being negative.

> Real question: is there some hidden purpose behind this kind of logic?

The basic strategy is to double the current capacity unless that will 
trigger an unnecessary exception, in which case just use the requested 
capacity, but again watch for the implementation limits.

Cheers,
David
-----

> 
> Cheers,
> 
> -- Jim
> 
>