RFR: jsr166 jdk integration 2018-05

[David Holmes]

I'm still with Martin on this. It makes no sense to me to allow 
replacing one element to not cause CME, but replacing more than one does 
cause CME. This is inconsistent and confusing and the end result is the 
programmer won't know what to expect when or why.

The original intent of CME, from my recollections back in 
lead-up-to-Java-5 days, was to prevent iterators from breaking i.e. 
throwing exceptions, due to the occurrence of the "concurrent" operation 
that changed the structure. It was not intended as an indicator of a 
semantic programming error. Replacing one element whilst there is a live 
iterator can be just as semantically wrong as replacing them all.

Cheers,
David
-----

On 16/05/2018 10:34 AM, Stuart Marks wrote:
>> (TL;DR - replaceAll incrementing modCount is a bug.)
> 
> I acknowledge that statement is the one in dispute.
> 
>> Hmmm ... my previous convincing arguments have failed to convince ?!
>>
>> Your argument above applies to List.set just as much as 
>> List.repladeAll, because the latter is nothing more semantically than 
>> a bunch of calls to the former. They should have the same behavior.  
>> Not having the same behavior leads to inconsistency, seen today in 
>> subList operations on ArrayList and Vector having different modCount 
>> behavior than on the root list.
> 
> Right, I read those arguments, and I'm not convinced.
> 
> Just because an individual operation has some characteristic doesn't 
> necessarily imply that an aggregate operation must have the same 
> characteristic. (This is variously called a fallacy of composition, or 
> in U.S. tax law, the step transaction doctrine.)
> 
> Bringing replaceAll() to the public API exposes it as a single 
> operation, with its own semantics. Note that Vector.replaceAll() holds 
> the lock for the entire operation, not merely around individual set() 
> operations. It really is different from performing individual set() 
> operations.
> 
>> Again, imagine this use case: there is a periodic background task that 
>> optimizes all the elements of a Vector
>> vector.replaceAll(x -> optimized(x))
>> That should not break any iterations in progress.
> 
> I don't think it's possible to do that correctly without holding a lock 
> around the entire iteration. If the lock is held, CME can't occur, as a 
> concurrent replaceAll() will occur before or after the iteration, never 
> in the middle.
> 
> If an iteration over a Vector doesn't hold a lock, any read-modify-write 
> operations (consider a loop with a ListIterator on which set() is 
> called) can be interleaved with bulk operations (like replaceAll) which 
> is clearly incorrect. In such cases, CME should be thrown.
> 
> Also, this use case cannot be written today, because CME is thrown. I'm 
> not aware of an actual use case that's been prevented because CME is 
> thrown. Of course, as API designers we have to anticipate needs and not 
> just react to complaints. My view of this kind of situation 
> (interleaving of bulk operation(s) with an iteration loop) is much more 
> likely to be a programming error than an actual use case.
> 
>> To strengthen that, the default method List.replaceAll is specified to 
>> be equivalent to
>>      final ListIterator<E> li = list.listIterator();
>>      while (li.hasNext()) {
>>          li.set(operator.apply(li.next()));
>>      }
>>
>> and incrementing modCount breaks "equivalent to".
> 
> Note carefully: that is an "Implementation Requirement" on the default 
> implementation of List.replaceAll(). It doesn't govern implementations 
> in implementing classes such as ArrayList.
> 
> s'marks