On Thu, 16 Jan 2025 00:07:49 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Passing a `VetoableListDecorator.subList()` to any of its bulk operations
>> (`addAll`, `setAll`, `removeAll`, `retainAll`) throws
>> `ConcurrentModificationException`. The reason is that the
>> `VetoableListDecorator.modCount` field is incremented before the underlying
>> list's bulk operation is invoked, which causes a mismatch when the sublist
>> is interrogated by the bulk operation.
>>
>> However, simply updating the `modCount` field _after_ the underlying list
>> was modified also doesn't work, as in this case listeners can't see the
>> correct value for `modCount` in their callback. The fix is to make a
>> defensive copy of the sublist before invoking the underlying list's bulk
>> operation.
>
> modules/javafx.base/src/main/java/com/sun/javafx/collections/VetoableListDecorator.java
> line 391:
>
>> 389:
>> 390: /**
>> 391: * Returns the specified collection as an unmodifiable list that
>> can safely be used in all bulk
>
> Do you think it might be easier to create a defensive copy **always**?
>
> In other words, can we guarantee that it is impossible for the user to create
> a convoluted code involving maybe two `VetoableListDecorators` where the
> second one loops back the changes to the first one, however ridiculous that
> might sound?
The way I see it, the situation that erroneously triggers
`ConcurrentModificationException` only happens when `VetoableListDecorator`
accesses its own sublist:
try {
modCount++;
boolean ret = list.addAll(index, c); // --> c is its own sublist
...
Since `modCount` is modified first, and the sublist refers back to the same
modified `modCount`, the exception occurs. It can't occur when we are dealing
with another list (or a sublist of another list), since in this case there is
no self-referential conflict.
The way `ArrayList` circumvents this problem is by incrementing `modCount` only
after the operation is done, not before it has started; it doesn't create a
defensive copy.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1679#discussion_r1917507388
