RFR(s): 8203670: unmodifiable List iterator() implementations should not be ListIterators

[Stuart Marks]

On 6/26/18 9:57 AM, Ivan Gerasimov wrote:
> On the first place, I'd like to understand why it is bad, if 
> List.of().iterator() in fact returns ListIterator?
> What kind of problems it may cause?

Basically it leaks information. Somebody might get an iterator, partially 
advance it, and hand it off to another piece of code, assuming that it can only 
be advanced. But if that instance can be downcast and used as a ListIterator, 
that assumption would be violated. I admit it's a narrow case, but it's 
nonetheless a possibility.


> Out of curiosity.  What if someone does something like
> 
> if (it instanceof ListIterator) {
>     // optimized for bidirectional access
> } else {
>     // slower algorithm
> }
> 
> previous(), nextIndex() and previousIndex() methods are not declared to be optional, so is it appropriate to throw UnsupportedOperationException from them?
> 
> Someone may assume that if an object can be cast to ListIterator interface, non-optional methods should not throw UOE.

I think this usage is out of bounds for the List API. The List specification is:

     Iterator<E> iterator();
     ListIterator<E> listIterator();
     ListIterator<E> listIterator(int);

That is, you get an Iterator from the iterator() method, and you get a 
ListIterator from the listIterator() methods. This implementation fulfils all 
the contracts for the interfaces declared as the return types. In particular, if 
you ask for a ListIterator, you get a ListIterator implementation that 
implements all the methods.

However, if you call iterator() and get an Iterator, and do an 'instanceof' and 
downcast, then all bets are off. It's reasonable to rely on the thing returned 
by iterator() having any behaviors other than those specified by Iterator.


Peter Levart wrote:

> Also, performance would be better if it was a separate class. It could be a superclass of ListItr so that common logic is not duplicated.

This isn't at all obvious. Certainly it's not obvious enough to embark on a 
refactoring without doing some benchmarking.

However, I want to fix this bug in JDK 11. If somebody wants to do some 
benchmarking, they're welcome to do so, but I don't want to consider this as 
part of this changeset.

> @Stable annotation on private final boolean isListIterator does not help here unless the returned Iterator object is assigned to a static final field and used from it. Hardly a common use case for Iterator(s) I guess.

The exact effect of @Stable is Hotspot-specific and changes over time. Here 
we're using it as a declaration that this final field really can be treated as 
final, and that it won't be modified via reflection or a varhandle, thus 
enabling VM more aggressive optimizations.

s'marks