RFR: 8079136: Accessing a nested sublist leads to StackOverflowError

[Ivan Gerasimov]

Thanks Tagir for the comments!


On 02.02.2016 9:19, Tagir F. Valeev wrote:
> Hello!
>
> IG> It's misfortune that the spec of subList() doesn't match the spec of
> IG> similar methods, like CharSequence.subSequence() or String.substring().
> IG> It even contradicts the spec of List.subList(), which declares only IOOB
> IG> to be thrown!
>
> Yes, it's a pity. Isn't it the reason to change (fix) the spec making
> List.subList and AbstractList.subList conformant?
A request to fix that is already there:
https://bugs.openjdk.java.net/browse/JDK-4506427 (it's 15 years old!)

I'd rather keep it separate from this fix (JDK-8079136).


>>> AbstractList::rangeCheck could be replaced with Obejcts::checkIndex.
>>> The same for ArrayList class.
> IG> Right.
> IG> Here's the updated webrev:
> IG> http://cr.openjdk.java.net/~igerasim/8079136/06/webrev/
>
> I have a doubt about replacing rangeCheckForAdd. What if size() ==
> Integer.MAX_VALUE? This is not an issue for ArrayList as it's limited
> by MAX_ARRAY_SIZE which is Integer.MAX_VALUE - 8. However for user
> defined AbstractList having size = Integer.MAX_VALUE looks possible.
> In this case range check would fail for any index.

Ah, good point.
I'll revert that part that part of the change back.

Sincerely yours,
Ivan


> With best regards,
> Tagir Valeev.
>
> IG> Sincerely yours,
> IG> 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
>>>
>>>
>