RFR: JDK-8146568 NegativeArraySizeException in ArrayList.grow(int)

[Stuart Marks]

Hi Martin,

Thanks for digging into this. There are some subtle issues here. I have a few 
questions.

One is that in list.addAll(other), the sizes of list and other exceeds 
Integer.MAX_VALUE, then grow(int) will be called with a negative value for 
minCapacity. The code *seems* to handle this ok, but the negative minCapacity 
value can get pretty deeply into the helper methods before being caught. Would 
it be better to check it at the top of grow(int) and throw an exception there? 
(Probably OOME.) I think it would make the subsequent code easier to follow.

It looks like there are a variety of ways for minCapacity that is positive but 
greater than MAX_ARRAY_SIZE to reach newCapacity(). If this occurs, and other 
conditions are right, it looks like the code will end up attempting to allocate 
an array greater than MAX_ARRAY_SIZE.

One style point I noticed (which might be an issue of me not being used to it) 
is the use of an elementData local variable to shadow the elementData field. 
This is more-or-less ok in places where elementData is initialized from 
this.elementData, but in readObject(), the local elementData is introduced in a 
nested scope. This means that elementData has different meanings in different 
parts of the method.

For the test Bug8146568 I think the preferred way to disable a test with extreme 
memory requirements is to use @ignore; see

    jdk/test/java/math/BigInteger/SymmetricRangeTests.java

for example.

s'marks

On 1/21/16 8:38 AM, Martin Buchholz wrote:
> PIng.  How about Alan, Chris, Xueming ?
>
> On Tue, Jan 19, 2016 at 9:24 AM, Martin Buchholz <martinrb at google.com> wrote:
>> Hi Stuart et al,
>> Please review my fix for this corner case bug, including more
>> importantly some performance improvements.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8146568
>> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/ArrayList-grow/