RFR: 8079136: Accessing a nested sublist leads to StackOverflowError

[Ivan Gerasimov]

Thank you Tagir for the review!

On 29.01.2016 7:16, Tagir F. Valeev wrote:
> Hello!
>
> AbstractList::subListRangeCheck could be replaced with new
> Objects::checkFromToIndex for consistency with other methods. Also
Yes, that would be good.
However, for AbstractList.subList() we have this in spec:
      * @throws IllegalArgumentException if the endpoint indices are out 
of order
      *         {@code (fromIndex > toIndex)}

but the 2-args checkFromToIndex() will throw IndexOutOfBoundsException 
instead.

If we use 3-args checkFromToIndex, it wouldn't save us anything in the 
terms of lines of code.

It's misfortune that the spec of subList() doesn't match the spec of 
similar methods, like CharSequence.subSequence() or String.substring().
It even contradicts the spec of List.subList(), which declares only IOOB 
to be thrown!


> AbstractList::rangeCheck could be replaced with Obejcts::checkIndex.
> The same for ArrayList class.

Right.
Here's the updated webrev:
http://cr.openjdk.java.net/~igerasim/8079136/06/webrev/

Sincerely yours,
Ivan

> Otherwise looks good to me.
>
> With best regards,
> Tagir Valeev.
>
> IG> Hello everyone!
>
> IG> I'd like to respin the discussion.
> IG> The previous attempt can be found here:
> IG> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-May/033159.html
>
> IG> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8079136
> IG> WEBREV: http://cr.openjdk.java.net/~igerasim/8079136/05/webrev/
>
> IG> Here's the summary of the proposed changes:
> IG> 1)
> IG> Sublist of an AbstractList (AbstractList.SubList class) now maintains a
> IG> link to the root AbstractList, and not only to the immediate parent list.
> IG> This allows to perform modifications on the chain of sub-lists in a loop
> IG> instead of using recursion (which, in particular, helps avoid the
> IG> stack-overflow problem).
> IG> The sublist is still backed by its parent list, in sense that all the
> IG> modifications are correctly reflected in the backing list (as well as in
> IG> all the grand parents the sublist, if any), so the fix does not violate
> IG> the existing specification.
>
> IG> 2)
> IG> It is proposed to update the spec of AbstractList.subList() in the
> IG> @implSpec section by removing the words about private fields.
> IG> With the fix, some of those private fields are removed.
>
> IG> 3)
> IG> A similar change is proposed for the ArrayList.SubList class.
>
> IG> 4)
> IG> Two regression tests are added:
> IG> NestedSubList.java - demonstrates a stack-overflow problem when dealing
> IG> with a long chain of sublists,
> IG> SubList.java - tests basic functionality of sub-lists, which helps us
> IG> make sure nothing is broken with the proposed change.
>
> IG> If people agree that the fix is good, I'll file a CCC request for
> IG> changing the spec of AbstractList.subList().
>
> IG> Sincerely yours,
> IG> Ivan
>
>