RFR: 8079136: Accessing a nested sublist leads to StackOverflowError

[Doug Lea]

On 05/06/2015 07:23 PM, Martin Buchholz wrote:
> Hi Ivan,
>
> I'm afraid of these changes - they are hard to review.


I believe that they also break the AbstractList.subList spec.
http://docs.oracle.com/javase/8/docs/api/java/util/AbstractList.html#subList-int-int-

The reason that flattened forms were not used is that each
layer does not in general know the class of its parent.

However, flattened forms are used in java.util.ArrayList.SubList.

It would be possible (and easy) to create a specialization for the
java.util.Arrays.ArrayList class (i.e., the kind returned by
Arrays.asList(a).subList), which would also fix the SOE problem
in this particular case.


-Doug



>
> Can't we fix the SOE with a relatively small change to ArrayList.SubList
> methods that recursively invoke parent methods to use iteration instead,
> i.e. can't you implement updateSizeAndModCount in the existing SubList
> class?
>
> ---
>
> I would make modCount an argument to updateSizeAndModCount.
>
> ---
>
> Separate out hot and cold code in subListRangeCheck (although pre-existing
> code had the same problem)
>
> +    static void subListRangeCheck(int fromIndex, int toIndex, int size) {
> +        if (fromIndex < 0)
> +            throw new IndexOutOfBoundsException("fromIndex = " +
> fromIndex);
> +        if (toIndex > size)
> +            throw new IndexOutOfBoundsException("toIndex = " + toIndex);
> +        if (fromIndex > toIndex)
> +            throw new IllegalArgumentException("fromIndex(" + fromIndex +
> +                                               ") > toIndex(" + toIndex +
> ")");
> +    }
> +
>
> if ((fromIndex < 0) | (toIndex > size) | (fromIndex > toIndex)) slowpath();
>
> ---
> java style consensus has been converging on: java source files should have
> exactly one top-level class.  It seems like you changed inner class SubList
> to be a top-level class - why?
>
> +class ArraySubList<E> extends AbstractList<E> implements RandomAccess {
>
>
>
>
>
> On Wed, May 6, 2015 at 1:25 PM, Ivan Gerasimov <ivan.gerasimov at oracle.com>
> wrote:
>
>> And here's another update:
>>
>> WEBREV: http://cr.openjdk.java.net/~igerasim/8079136/2/webrev/
>>
>