On Tue, May 15, 2018 at 10:19 AM, Stuart Marks <stuart.ma...@oracle.com> wrote:
> > > I mentally revised the history when doing the collections/stream API work >>>>>> since we added more bulk operations, since this is on a “best effort” >>>>>> basis >>>>>> and if it’s cheap to do then there is no real harm in it and it might >>>>>> help. >>>>>> >>>>>> Spec says: >>>>>> >>>>>> """protected transient int modCount >>>>>> >>>>>> The number of times this list has been structurally modified. >>>>>> 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.""" >>>>>> >>>>>> replaceAll doesn't qualify as a structural modification. >>>>>> >>>>> >>>>> Why not? It can "perturb it in such a fashion that iterations in >>>>> progress may yield incorrect results”. >>>>> >>>>> Why? It replaces every element "inplace" in the style of List.set(i) >>>>> which is also not a structural modification. >>>>> >>>> I get that, but I would argue that (placing the implementation aside) >>>> the bulk operation as a whole is compatible with the “otherwise” part of >>>> the modCount definition, since it can perturb the list and effect the yet >>>> to be traversed elements. Such usage is a likely source of hard to track >>>> down bugs that CMEs are designed to help flag. >>>> I doubt i am gonna change your mind on this :-) So we may just have to >>>> agree to disagree on the interpretation of the definition and move on. I >>>> would prefer it remains but it's your call. >>>> >>> >>> FWIW I agree with Martin - sorry Paul :) >>> >> >> That’s ok. Its clear i am out numbered here, and overspent my budget on >> debating CME for the month :-) >> > > Don't give up so quickly, Paul. :-) > > I remember working on the bulk operations back in JDK 8 (with Mike > Duigou?) and we discussed the issue of bulk operations vs. concurrent > modification. > > Even though operations like replaceAll() and sort() don't "structurally > modify" the list, our interpretation was that they did perturb in-progress > iterations. These operations potentially change every element, thus an > in-progress iteration is likely to observe some unspecified mixture of the > "before" and "after" states of the list. That sounds like a programming > error. The intent of CME is to prevent programming errors, so we focused on > that instead of on "structural modification." The "perturb" clause seemed > to cover this behavior, so we left that part of the spec unchanged. > > Note that in JDK 8, both ArrayList's and Vector's replaceAll() both will > cause CME to be thrown by an in-progress iteration. I think the onus should > be on Martin to show why replaceAll() should no longer trigger CME. > > (TL;DR - replaceAll incrementing modCount is a bug.) 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. 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 also lean to removing the modCount increment in sort, but there the argument is much weaker, since that likely WILL perturb any iteration in progress) > An alternative would be to adjust the specification of modCount to clarify > that it accommodates bulk modifications that aren't strictly structural > modifications. > > s'marks >
