RFR: 8079136: Accessing a nested sublist leads to StackOverflowError

Tagir F. Valeev amaembo at gmail.com
Tue Feb 2 06:19:10 UTC 2016


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?

>> 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.

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
>>
>>




More information about the core-libs-dev mailing list