inconsistency between ArrayList.modCount and logic of ArrayList::addAll

[Stuart Marks]

On 5/8/19 7:44 AM, Pavel Rappo wrote:
> We will certainly hear from Stuart on this very soon.

Heh.

On 5/8/19 5:55 AM, Сергей Цыпанов wrote:
> javadoc for AbstractList.modCount is described as 
> 
>> The number of times this list has been <i>structurally modified</i>.
>> Structural modifications are those that change the size of the
>> list, or otherwise perturb it in such a fashion that iterations in
>> progress may yield incorrect results.
> 
> However when we execute this
> 
> ------------------------
> ArrayList<Object> objects = new ArrayList<>();
> boolean result = objects.addAll(Collections.emptyList());
> ------------------------
> 
> modCount is 1, not 0 while result is false. I.e. returned value claims the list is not modified, but the inner state of the same list demonstrates the opposite.

I think Pavel has the right point here, which is whether modCount should be 
incremented if the list is *actually* modified or whether an *attempt* to modify 
it is made. On the one hand, a strict reading seems to indicate incrementing 
only actual modifications. However, if the goal is to catch programming errors, 
then incrementing on any modification *attempt* seems preferable.

I described this as "state-dependent" behavior in a message in this recent thread:

     https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059708.html
     https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/059979.html

Briefly, consider code like

     for (Object obj : objects) {
         ...
         objects.addAll(getAdditionalObjects());
         ...
     }

This is clearly bad code because it modifies the list during iteration. We want 
ConcurrentModificationException to be thrown regardless of whether 
getAdditionalObjects() returns an empty collection.

In general, the incrementing of modCount (and the consequential throwing of CME) 
is an approximation. We usually want to it catch as many errors as feasible, but 
not at the cost of unreasonable performance or code complexity.

s'marks