RFR: 8347753: VetoableListDecorator doesn't accept its own sublists for bulk operations

[Andy Goryachev]

On Wed, 15 Jan 2025 00:32:37 GMT, Michael Strauß <mstrauss at 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.

The scenario I was referring to is impossible, I think.

One possible alternative is to create the defensive copy each time, this will save one extra pointer every time an iterator or a sublist gets created (these objects might be long lived).  The code in this PR creates a copy in many (most?) cases anyway, and in my opinion, the memory is more precious resource that CPU cycles (i.e. using extra memory costs many more CPU cycles in garbage collection etc.), so please consider that.

+ some minor suggestions

modules/javafx.base/src/test/java/test/javafx/collections/VetoableObservableListTest.java line 212:

> 210:         list.addAll(list.subList(0, 2));
> 211:         assertSingleCall(new String[] {"foo", "bar"}, new int[] {4, 4});
> 212:     }

suggestion: also check that the list contains the newly added elements?
(here and in added tests that involve subList?)

-------------

PR Review: https://git.openjdk.org/jfx/pull/1679#pullrequestreview-2556899713
PR Review Comment: https://git.openjdk.org/jfx/pull/1679#discussion_r1918996891