Sometimes constraints are questionable

[Jim Laskey]

My mistake it's ArraysSupport.newLength that's failing here,

String::replace

                pos = Arrays.copyOf(pos, ArraysSupport.newLength(p, 1, p >> 1));

newLength should be causing the exception but it in turn is passing the OOM size on to Arrays.copyOf

The logic in ArraysSupport::newLength is similar to that in the first part of this discussion:

    public static int newLength(int oldLength, int minGrowth, int prefGrowth) {
        // assert oldLength >= 0
        // assert minGrowth > 0

        int newLength = Math.max(minGrowth, prefGrowth) + oldLength;
        if (newLength - MAX_ARRAY_LENGTH <= 0) {
            return newLength;
        }
        return hugeLength(oldLength, minGrowth);
    }

    private static int hugeLength(int oldLength, int minGrowth) {
        int minLength = oldLength + minGrowth;
        if (minLength < 0) { // overflow
            throw new OutOfMemoryError("Required array length too large");
        }
        if (minLength <= MAX_ARRAY_LENGTH) {
            return MAX_ARRAY_LENGTH;
        }
        return Integer.MAX_VALUE;
    }

That is, pass Integer.MAX_VALUE on to the allocator and let it fail.


> On Jun 1, 2020, at 8:56 AM, Jim Laskey <james.laskey at oracle.com> wrote:
> 
> In the same light the test in String::replace
> 
>        int resultLen;
>        try {
>            resultLen = Math.addExact(valLen,
>                    Math.multiplyExact(++p, replLen - targLen));
>        } catch (ArithmeticException ignored) {
>            throw new
>            OutOfMemoryError("String resulting from replace exceeds maximum string size");
>        }
> 
> Is useless in this situation (from the bug report):
> 
>        int missingToIntMax = 10;
>        String tooLarge = "a".repeat(Integer.MAX_VALUE - missingToIntMax);
>        tooLarge.replace("a", "a".repeat(missingToIntMax + 2));
> 
> The replace "test" does not fail and the exception comes from further down the stack:
> 
> Exception in thread "main" java.lang.OutOfMemoryError: Java heap space
> 	at java.base/java.util.Arrays.copyOf(Arrays.java:3584)
> 	at java.base/java.lang.StringLatin1.replace(StringLatin1.java:349)
> 	at java.base/java.lang.String.replace(String.java:2173)
> 	at JI9062163.main(JI9062163.java:6)
> 
> 
> 
> 
>> On May 31, 2020, at 10:15 PM, Martin Buchholz <martinrb at google.com> wrote:
>> 
>> I'm still not deeply reading the code in java.nio but I see in
>> AbstractStringBuilder
>> 
>>    * Will not return a capacity greater than
>>    * {@code (MAX_ARRAY_SIZE >> coder)} unless the given minimum capacity
>>    * is greater than that.
>> 
>> My intent was that a garden variety grow method would first grow to
>> MAX_ARRAY_SIZE, then to Integer.MAX_VALUE.
>> 
>> (this doesn't always work, e.g. hash tables want to have a
>> power-of-two sized backing array)
>> 
>> (these methods have evolved recently without much help from me)
>> 
>> (Library code should know as little about VM implementation
>> idiosyncrasies as possible)
>> 
>> On Sun, May 31, 2020 at 5:56 PM David Holmes <david.holmes at oracle.com> wrote:
>>> 
>>> On 1/06/2020 9:52 am, Martin Buchholz wrote:
>>>> On Sun, May 31, 2020 at 4:35 PM David Holmes <david.holmes at oracle.com> wrote:
>>>> 
>>>>> 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.
>>>> 
>>>> At least some of the grow methods were designed so that callers did
>>>> not have to do the checking, and grow would interpret negative values
>>>> as int overflow
>>>> 
>>>>>> 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.
>>>> 
>>>> Again, the original design was to allow a capacity with MAX_ARRAY_SIZE
>>>> < capacity <= Integer.MAX_VALUE if and when the VM also allowed it.
>>> 
>>> The only way to know that is to try and create the array, catch the OOME
>>> and then adjust the value used. None of the code does that so I can't
>>> see how your claim here can be correct. My understanding, having been on
>>> the VM side, was that Hotspot's internal limits were reflected on the
>>> Java side so that the Java code could avoid the VM exception.
>>> 
>>> David
>